Skip to content

Add dots3-note Preview model support - #47844

Open
miraclezqc wants to merge 26 commits into
huggingface:mainfrom
miraclezqc:add-dots3-note-omni
Open

miraclezqc wants to merge 26 commits into
huggingface:mainfrom
miraclezqc:add-dots3-note-omni

Conversation

@miraclezqc

@miraclezqc miraclezqc commented Aug 8, 2026 •

Copy link
Copy Markdown

CPU CI GPU run-slow

What does this PR do?

This PR is submitted by the official Dots-Studio team.

This PR adds native Transformers support for Dots 3 Note Preview, an inference-only mixture-of-experts multimodal causal language model supporting text, image, video, and audio inputs.

The implementation follows the modular Transformers workflow and includes the model configuration, processor, image processor, audio feature extractor, modeling implementation, auto-class registrations, documentation, conversion utilities, and model tests required for native integration.

The implementation includes:

  • text, image, video, and audio input processing;
  • greedy multimodal generation;
  • sliding-window attention and DSA indexer inference;
  • multimodal cache handling during generation;
  • variable-length left-padded batching;
  • BF16 checkpoint support;
  • fine-grained FP8 checkpoint support using dynamic activations, E4M3 weights, and a 128 x 128 weight block size;
  • FP32 routing for the vision MoE router.

This integration currently targets inference. Training-specific features such as gradient checkpointing are outside the scope of this PR.

No additional runtime dependency is introduced by this integration.

Validation

The following checks were run against the implementation in this PR:

  • the complete tests/models/dots3_note test directory;
  • modular-to-generated modeling conversion and synchronization checks;
  • Ruff formatting and lint checks;
  • repository diff checks;
  • clean loading of the real BF16 and FP8 checkpoints;
  • greedy end-to-end generation through AutoModelForMultimodalLM;
  • text reasoning tests, including GSM8K-style problems;
  • speech transcription;
  • image question answering;
  • video understanding;
  • multimodal cache and masking regression tests;
  • FP8 dequantization with non-divisible weight dimensions;
  • DSA projection precision and derived-weight cache invalidation;
  • vision MoE near-tie routing precision.

Both the BF16 and FP8 checkpoints passed the end-to-end text, image, video, and audio validation cases with finite outputs and successful greedy generation.

The corresponding public checkpoint links and finalized model organization metadata will be added after this PR is merged.

Code Agent Policy

The Transformers repo is currently being overwhelmed by a large number of PRs and issue comments written by
code agents. These often are low-quality, or fix extremely minor issues that occur rarely or never in practice.
As a result, we're instituting a rule that first-time contributors should not use code agents to submit PRs or issues.
We'd also ask autonomous "OpenClaw"-like agents not to open any PRs or issues.

Issues/PRs from first-time contributors that violate this rule will probably just be closed without review, and we
might block you, especially if you open more than one or appear to be deliberately ignoring this. We especially do not
want new contributors to jump in on random issues to contribute an agent-written fix. This creates lots of noise
for reviewers and other users and will almost certainly get you blocked.

For more information, please read CONTRIBUTING.md.

  • (First-time contributors only): I confirm that this PR description and code is not written by an LLM or code agent

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline and the Pull Request checks?
  • Was this discussed/approved via a GitHub issue or the forum? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes according to the guidelines?
  • Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed.

cc @zucchini-nlp @vasqu

@miraclezqc
miraclezqc marked this pull request as ready for review August 8, 2026 13:21
@miraclezqc miraclezqc changed the title Add dots3 note omni Add Dots3-Note Omni model support Aug 8, 2026
@miraclezqc
miraclezqc force-pushed the add-dots3-note-omni branch from 7af5a25 to fe87862 Compare August 9, 2026 05:45
@miraclezqc miraclezqc changed the title Add Dots3-Note Omni model support Add Dots 3 Note Preview model support Aug 10, 2026
@zucchini-nlp

Copy link
Copy Markdown
Member

hey @miraclezqc , thanks for the PR! Most of the team currently is off on summer vacations so the reviews are taking longer. We'll review it soon :)

@miraclezqc
miraclezqc force-pushed the add-dots3-note-omni branch from b70c2d9 to d8e147f Compare August 13, 2026 10:27
@miraclezqc

miraclezqc commented Aug 13, 2026 •

Copy link
Copy Markdown
Author

hey @miraclezqc , thanks for the PR! Most of the team currently is off on summer vacations so the reviews are taking longer. We'll review it soon :)

@zucchini-nlp
Thanks for the update—we completely understand and look forward to your comments when the team has time to review!

@miraclezqc
miraclezqc force-pushed the add-dots3-note-omni branch from 84acd71 to 9daa866 Compare August 13, 2026 20:06
@miraclezqc miraclezqc changed the title Add Dots 3 Note Preview model support Add dots3-note Preview model support Aug 14, 2026
@ngxson ngxson mentioned this pull request Aug 14, 2026
3 of 4 tasks
@ArthurZucker

Copy link
Copy Markdown
Collaborator

Hey! Happy to review, can you update the branch to fix the latest mlinter updates!

@miraclezqc

Copy link
Copy Markdown
Author

Hey! Happy to review, can you update the branch to fix the latest mlinter updates!

@ArthurZucker

Thanks! The branch has been updated for the latest mlinter rules.

It seems that the new CI run is blocked by the security gate. Could you please approve it?

@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@github-actions

Copy link
Copy Markdown
Contributor

CI recap

Dashboard: View test results in Grafana
Latest run: 32138376283:2
Result: success | Jobs: 16 | Tests: 182,471 | Failures: 1 | Duration: 12h 28m

@miraclezqc

Copy link
Copy Markdown
Author

Thanks for approving the workflow! All regular CI checks are now green, and the branch is ready for review.
We’d appreciate your review when convenient and will address any feedback promptly.

@miraclezqc

Copy link
Copy Markdown
Author

Hi @ArthurZucker @Rocketknight1 , the branch is now up to date with main, and all relevant local tests pass. We’d appreciate your review and an early merge if everything looks good. Thank you!

@vasqu

vasqu commented Sep 4, 2026 •

Copy link
Copy Markdown
Collaborator

Sorry for the delays @miraclezqc but I noticed that we are not using modular as intended at all, please check out https://huggingface.co/docs/transformers/modular_transformers

The goal is to reuse existing modules as much as we can, this includes the rms norm, indexer, etc

@miraclezqc

Copy link
Copy Markdown
Author

@vasqu Thanks! We’ve refactored the model to reuse existing DeepSeek, GLM DSA, and Qwen2-VL components, including RMSNorm and the indexer. Only Dots-specific logic remains custom, and all relevant tests pass.

Could you please take another look and let us know if it is ready to merge?

@vasqu vasqu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to go a bit more into details but this is still not super modular friendly: There are a lot of custom implementations where we could just reuse ours - e.g. the dsa is handled completely custom, the attn backend is also completely custom which would disallow any usage via vllm, etc.

What I want to tell with this: Custom implementations are very much still implemented in transformers oftentimes but they need to be aligned - imo the biggest example is RoPE in this case

Comment thread docs/source/en/model_doc/dots3_note.md Outdated
Comment on lines +31 to +36
> [!NOTE]
> Loading encoded audio or video sources requires the optional `torchcodec` dependency. Native video preprocessing
> follows the training-time sampling pipeline and expands each video into timestamped image blocks with interleaved
> audio blocks. The `<|video_pad|>` marker is only an external prompt placeholder and is removed before tokenization;
> it is never passed to the model. Decoded frame arrays are supported as visual-only inputs because their original
> audio track is absent.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fairly common across all of our models so I think we can delete it but no strong opinion

Comment thread docs/source/en/model_doc/dots3_note.md
Comment thread docs/source/en/model_doc/dots3_note.md
return model


def _resolve_weight_block_size(hf_quantizer, value: torch.Tensor) -> tuple[int, int]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks unnecessary no? Can we revert to keep it inline and not a separate fn?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @IlyasMoutawwakil when you have time to check this over

can you share the motivation here? Ig there is some (new) fp4 handling we need?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still need an answer here, seems like something happens alongside the sharding so we don't have enough to properly unpack?

we also want to move to a more uniform api in #48058 so would help which fp4 format we have here

pass


class Dots3NoteVisionRotaryEmbedding(nn.Module):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check qwen2 vl rope

return attention_output, attention_weights


class Dots3NoteVisionAttention(nn.Module):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here a lot of custom stuff we should not use and try to align with existing models

# Unified multimodal model
# -----------------------------------------------------------------------------
@auto_docstring
class Dots3NoteForCausalLM(Dots3NoteTextForCausalLM):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ForConditional - pls check the pattern in qwen vl

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or gemma4 and the like (omni)



@auto_docstring
class Dots3NoteForConditionalGeneration(Dots3NoteForCausalLM):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesnt make much sense to make for causal lm and for conditional

Is the goal to have a text only variation, then pls check out qwen3_5 which does that

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modular can also be applied for all processor related things

Didnt go through these but we have a lot of custom things we shouldnt need

@miraclezqc

Copy link
Copy Markdown
Author

I tried to go a bit more into details but this is still not super modular friendly: There are a lot of custom implementations where we could just reuse ours - e.g. the dsa is handled completely custom, the attn backend is also completely custom which would disallow any usage via vllm, etc.

What I want to tell with this: Custom implementations are very much still implemented in transformers oftentimes but they need to be aligned - imo the biggest example is RoPE in this case

Thanks @vasqu for the detailed review and all the helpful suggestions. We’ve gone through the comments and refactored the text, audio, vision, and preprocessing code to reuse existing Transformers components, removing redundant implementations along the way.

We believe most suggestions are addressed. The remaining differences and follow-ups are:

  • Shared MoE change: removed from this PR as requested. We will make a separate PR for this.
  • Shared FP8 changes: the remaining changes handle partial weight blocks, rather than adding FP4 support, and preserve converter scope/prefix information while handling already-anchored weight patterns. These are separate from the model-specific refactor.
  • Indexer: the custom projections and scoring implementation are removed in favor of GLM’s implementation. Loading-time dequantization of the original FP8 indexer weights/scales remains TODO.
  • Text attention: the custom forward retains Dots’ LoRA scaling, separate K-RoPE normalization, and output sigmoid gate, which are absent from the DeepSeek-V3.2 forward. The shared projections, KV expansion, and attention interface are reused.
  • Initialization and output handling: a small initializer remains for the per-layer-type RoPE buffers not restored by the DeepSeek implementation. Some model declarations and explicit attention/vision output handling also remain to preserve supported configurations and the existing output structure.
  • Model naming: text-only and multimodal classes are separated. The old Dots3NoteForCausalLM name remains only for compatibility with the original checkpoints, without a duplicate forward.
  • Preprocessing: image/video processing reuses Qwen components. Native-video audio/frame interleaving and joint token budgeting remain model-specific.

We may still have missed a comment or a simpler reuse opportunity. Could you take another look and let us know what else needs to change before merging? Thanks again for your time and guidance.

@vasqu

vasqu commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Checking tomorrow! 🤗

@vasqu vasqu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delays, another round 🫡

Comment thread docs/source/en/model_doc/dots3_note.md Outdated
Comment on lines +78 to +79
language-model head and generation interface. `Dots3NoteTextForCausalLM` provides the text-only variant;
`Dots3NoteForCausalLM` remains a compatibility name for the original multimodal checkpoints.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo its fine to have the for causal lm only variation, we dont really use textforcausallm

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still need an answer here, seems like something happens alongside the sharding so we don't have enough to properly unpack?

we also want to move to a more uniform api in #48058 so would help which fp4 format we have here

MISSING_VIDEO_PROCESSOR_MAPPING_NAMES = OrderedDict(
[
("cosmos3_omni", "Qwen3VLVideoProcessor"),
("dots3_note", "Dots3NoteVideoProcessor"),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is weird, should usually be detected into auto mappings no?

("dinat", {"torchvision": "ViTImageProcessor", "pil": "ViTImageProcessorPil"}),
("dinov2", {"torchvision": "BitImageProcessor", "pil": "BitImageProcessorPil"}),
("donut-swin", {"torchvision": "DonutImageProcessor", "pil": "DonutImageProcessorPil"}),
("dots3_note", {"pil": "Dots3NoteImageProcessor"}),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here should be detected in auto mappings

Comment thread src/transformers/models/auto/auto_mappings.py
pixel_values: torch.Tensor,
image_grid_thw: torch.Tensor,
**kwargs,
) -> torch.Tensor:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should return a model output class, check qwen vl

return output.audio_embeds, output.audio_token_lengths

@staticmethod
def _merge_multimodal_embeddings(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets use the existing placeholder mask pattern please

Comment on lines +1219 to +1223
chunk_sample_lengths (`torch.Tensor`, *optional*): Waveform sample count for each audio feature chunk.
chunk_token_lengths (`torch.Tensor`, *optional*): Encoder token count for each audio feature chunk.
audio_chunk_counts (`torch.Tensor`, *optional*): Number of feature chunks for each audio input.
audio_token_lengths (`torch.Tensor`, *optional*): Expected encoded token count for each audio input.
chunk_audio_indices (`torch.Tensor`, *optional*): Audio ownership metadata emitted by the processor.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea im still unsure here because this feels overkill and might be better described with metadata like mm token ids and cu seqlens etc?

if isinstance(image, Image.Image) and image.mode == "RGBA":
from ..idefics2 import image_processing_pil_idefics2

return image_processing_pil_idefics2.convert_to_rgb(image)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather we copy paste because i dont think modular would properly work here to unfold the implementation

if sample_end > sample_start:
waveform = np.ascontiguousarray(pcm[sample_start:sample_end].astype(np.float32) / 32768.0)
output.append({"type": "audio", "audio": waveform})
return output

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks way too custom, let's try to move it into our processor framework

E.g. video processor have sample frames functions we can override; loading any video should not be manually done as we load the video with metadata etc

@miraclezqc

Copy link
Copy Markdown
Author

Thanks @vasqu for another detailed review! We’ve pushed a further refactor.

One detail worth clarifying:

  • Text attention retains Dots3-specific Q/KV LoRA rescaling, separate K-RoPE normalization, and sigmoid gating. These aren’t provided by the DeepSeek parent, while the common components are reused.

Could you take another look? Thanks again for your patience and guidance!

@vasqu vasqu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next round but this looks very good already! I have smaller concerns but the overall core is solid

Comment on lines +28 to +31
pyramid_num_routed (`list[int]` or `tuple[int, ...]`, *optional*):
Number of routed experts in each vision transformer layer. A non-positive value selects a dense MLP.
capacity_factor (`int`, *optional*, defaults to 2):
Number of experts selected per token, capped by the layer's expert count.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to check the vision model in detail but why not use the same moe conventions as we have it in the text, e.g. num_experts_per_tok (capacity_factor) and mlp_layer_types (pyramid_num_routed)


model_type = "dots3_note_vision_encoder"
base_config_key = "vision_config"
attribute_map = {"num_heads": "num_attention_heads", "use_bias": "attention_bias"}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this attribute mapping? You can also let them live via kwargs being passed in post init (but we do not recognize it here)

if kwargs.pop("is_causal", False):
raise ValueError("Dots 3 Note Preview vision attention requires is_causal=False")
if not kwargs.pop("pre_pixel_shuffle", True) or kwargs.pop("adapter_type", "patch_merger") != "patch_merger":
raise ValueError("Dots 3 Note Preview requires pre_pixel_shuffle=True and adapter_type='patch_merger'")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo this is too overly cautious, let's just make sure our defaults are really fitting

if you have something that checks values, use validate_architecture

Comment on lines +126 to +131
attribute_map = {
"d_model": "hidden_size",
"encoder_attention_heads": "num_attention_heads",
"encoder_layers": "num_hidden_layers",
"encoder_ffn_dim": "intermediate_size",
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here if we can avoid using attribute mapping it would be nice

Comment on lines +182 to +185
self.head_dim = self.hidden_size // self.num_attention_heads
self.num_key_value_heads = self.num_attention_heads
self.hidden_act = "silu"
self.attention_bias = True

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bit weird, can be just set correctly as attribute no? (maybe happens in vision/text as well?)

Comment on lines +836 to +843
kwargs.update(
pixel_values=pixel_values,
pixel_values_videos=pixel_values_videos,
image_grid_thw=image_grid_thw,
video_grid_thw=video_grid_thw,
input_features=input_features,
chunk_sample_lengths=chunk_sample_lengths,
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's rather inherit from another omni model and avoid this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebump: no kwargs manipulation like this - rather write the complete forward again than this

return_attention_mask=return_attention_mask,
**kwargs,
)
self.dither = 0.0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm this is a bit weird no? why is it not just an arg as the others?

Comment on lines +982 to +992
# Correct legacy Qwen2-VL defaults without overriding custom pixel limits.
if image_processor is not None:
if dict(image_processor.size) == _QWEN2_VL_IMAGE_DEFAULT_SIZE:
image_processor.size = SizeDict(**dict(_RELEASE_VISION_SIZE))
if image_processor.temporal_patch_size == 2:
image_processor.temporal_patch_size = 1
if video_processor is not None:
if dict(video_processor.size) == _QWEN2_VL_VIDEO_DEFAULT_SIZE:
video_processor.size = SizeDict(**dict(_RELEASE_VISION_SIZE))
if video_processor.temporal_patch_size == 2:
video_processor.temporal_patch_size = 1

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo should be updated on the hub instead no? we shouldnt need that workaround


def convert_to_rgb(self, image):
return convert_to_rgb(image)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this needed

Comment thread src/transformers/models/dots3_note/modular_dots3_note.py
@miraclezqc

Copy link
Copy Markdown
Author

Thanks @vasqu! We’ve gone through your comments and pushed another round of changes. A few notes:

  • Text attention still keeps the Dots-specific Q/KV LoRA scaling, K-RoPE normalization, and gate, while reusing the common components.
  • The generation head now inherits from Voxtral; only a small adapter remains for the extra multimodal inputs.
  • The RGB override was originally for compositing transparent images onto a white background. We’ve removed it and now follow Qwen2-VL’s default behavior.

Could you take another look when you have a chance? We’d appreciate your feedback on anything we may have missed.

@vasqu vasqu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry that I'm doing so many comments again but we are definitely getting closer!

There are a lot of smaller details but overall the biggest thing is the tests now imo which should follow other similar models like gemma4 (so omni models)

Comment thread docs/source/en/model_doc/dots3_note.md Outdated
a shared vision encoder for images and videos and a Whisper-style audio encoder. Both encoders project their outputs
into the language model's hidden space before autoregressive text generation.

This integration supports BF16 checkpoints.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly the indexer or completely not usable? I think we dequant the indexer but the rest could use fp8 no?


def __post_init__(self, **kwargs):
if self.rope_parameters is None:
self.rope_parameters = {"rope_type": "axial", "rope_theta": 10_000.0}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just use default_rope_type="axial" instead then we dont need this

if pyramid_num_routed is None and self.mlp_layer_types is None:
pyramid_num_routed = [-1] * 25 + list(range(4, 65, 4)) + [64]
self.num_experts_per_tok = kwargs.pop("capacity_factor", self.num_experts_per_tok)
self.router_scaling_factor = kwargs.pop("router_scale", self.router_scaling_factor)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo you can just do get if you want these values to surive serialization

Comment on lines +158 to +159
if (attention_backend := kwargs.pop("attention_backend", None)) is not None:
kwargs.setdefault("attn_implementation", attention_backend)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not be done, we should rely on our set attn implementation etc and default to work out

self.num_key_value_heads = self.num_attention_heads
self.rope_parameters = {
"rope_type": "default",
"partial_rotary_factor": 0.5,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ig you only want the partial rotary factor, I think kwargs.setdefault("partial_rotary_factor", 0.5) should suffice

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or the set factor below

Comment on lines +161 to +162
# Initial support is inference-focused, so common coverage targets forward, cache, and generation.
self.is_training = False

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo can still be turned on, no resaon to turn it off. The indexer is the only part that has no proper aux loss but its known

# Initial support is inference-focused, so common coverage targets forward, cache, and generation.
self.is_training = False

def get_config(self):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed if we have a proper init override

Comment on lines +150 to +159
parent=parent,
batch_size=2,
seq_length=7,
vocab_size=128,
hidden_size=32,
num_hidden_layers=2,
num_attention_heads=4,
num_key_value_heads=4,
intermediate_size=64,
max_position_embeddings=128,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a lot of common kwarg like these are already covered. pls check e.g. youtu on the style

Comment on lines +177 to +179
all_model_classes = (Dots3NoteForCausalLM,) if is_torch_available() else ()
pipeline_model_mapping = {"text-generation": Dots3NoteForCausalLM} if is_torch_available() else {}
_is_stateful = True

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
all_model_classes = (Dots3NoteForCausalLM,) if is_torch_available() else ()
pipeline_model_mapping = {"text-generation": Dots3NoteForCausalLM} if is_torch_available() else {}
_is_stateful = True

we should mark stateful in pretrained model if anything but I doubt it



@require_torch
class Dots3NoteModelTest(unittest.TestCase):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's align with gemma4 style please

There are text only, vision+text, and audio+text tests similar to how you need it here + then finally a few integration tests

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution 🤗!

CI Security Gate — automatic approval blocked

This PR was not automatically approved for CI because the security gate failed.

Possible reasons:

  • The PR touches 50 or more files — only PRs with fewer than 50 changed files are automatically approved
  • A changed file is outside the allowed directories (src/, tests/, docs/, utils/), has a disallowed extension (only .py, .txt, .md permitted outside tests/ and docs/), or is not .md/.yml inside docs/ — this covers files the PR deletes or renames, not only the ones it edits
  • A new high-severity security issue was detected in the changed Python files (Bandit check)
  • The PR touches a path this repository protects from untrusted PRs, such as the file that decides who reviews it — a maintainer must make that change in a separate PR

See the workflow run for the exact violations.

A maintainer can review and manually approve CI if a finding is a false positive.

@github-actions

Copy link
Copy Markdown
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: auto, dots3_note

@miraclezqc

Copy link
Copy Markdown
Author

No worries at all, and thanks for your patience! I’m still getting familiar with Transformers conventions, which has made the iterations slower. A few points remain:

  • FP8 remains out of scope for this PR because selective indexer dequantization and numerical alignment, particularly on long sequences, still need validation, along with a full multimodal run.
  • The default vision path fails fullgraph capture at a data-dependent torch.split(..., lengths.tolist()) in the inherited GLM-OCR attention.
  • We now follow Gemma4’s subconfig construction pattern, but kept the flat-config fallback for released checkpoints. Removing it would lose checkpoint-specific values, such as the BF16 sliding window of 513. Would that be acceptable?

I’m happy to go through another round and keep iterating until everything is resolved.

@vasqu vasqu mentioned this pull request Oct 2, 2026
5 tasks
@miraclezqc

Copy link
Copy Markdown
Author

No worries at all, and thanks for your patience! I’m still getting familiar with Transformers conventions, which has made the iterations slower. A few points remain:

  • FP8 remains out of scope for this PR because selective indexer dequantization and numerical alignment, particularly on long sequences, still need validation, along with a full multimodal run.
  • The default vision path fails fullgraph capture at a data-dependent torch.split(..., lengths.tolist()) in the inherited GLM-OCR attention.
  • We now follow Gemma4’s subconfig construction pattern, but kept the flat-config fallback for released checkpoints. Removing it would lose checkpoint-specific values, such as the BF16 sliding window of 513. Would that be acceptable?

I’m happy to go through another round and keep iterating until everything is resolved.

Hi @vasqu, do you have any feedback on the latest changes?

@miraclezqc
miraclezqc requested a review from vasqu October 9, 2026 10:04

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants