microsoft / microsoft/winml-cli

Lift derived `head_dim` into a shared NormalizedConfig base class

Open
#401 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

P2 refactor triaged
Dominant language
Python
Stars
40
Forks
11
Avg merge
1d 8h
Merged PRs (30d)
50

Description

Context

Follow-up from PR #382 review — see this comment by @zhenchaoni.

Several HuggingFace model configs do not expose head_dim natively (e.g. BartConfig, MarianConfig, and per the comment also blip and trocr). To make these models work with PastKeyValueInputGenerator (which now reads normalized_config.head_dim unconditionally), PR #382 introduced per-model NormalizedConfig subclasses with a computed head_dim:

  • src/winml/modelkit/models/hf/bart.py_BartDecoderNormalizedConfig.head_dim
  • src/winml/modelkit/models/hf/marian.py_MarianDecoderNormalizedConfig.head_dim

Both implementations are identical:

@property
def head_dim(self) -> int:
    return self.hidden_size // self.num_attention_heads

Proposal

Lift the derived head_dim property into a shared NormalizedConfig base class (e.g. _DerivedHeadDimNormalizedConfig in a common module under models/winml/ or models/hf/) so that BART, Marian, and the upcoming BLIP / TrOCR configs can simply inherit it, instead of redefining the same property in each subclass.

Acceptance criteria

  • New shared base class with the derived head_dim property in a common location
  • _BartDecoderNormalizedConfig and _MarianDecoderNormalizedConfig inherit from it and drop their local head_dim overrides
  • BLIP and TrOCR (when added) reuse the same base class
  • Existing BART / Marian export tests still pass

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading src/winml/modelkit/models/hf/bart.py and src/winml/modelkit/models/hf/marian.py, then inspect the normalized-config modules under models/winml or models/hf for a suitable shared location. Extract the identical derived head_dim behavior into a shared base, update both decoder configs to inherit it, and run the existing BART and Marian export tests to confirm they still pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
machine-learning
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.