Skip to content

Verify PR #2078 builder design changes already present - #359

Closed
xadupre with Copilot wants to merge 1 commit into
mainfrom
copilot/bring-design-changes-from-pr-2078
Closed

Verify PR #2078 builder design changes already present#359
xadupre with Copilot wants to merge 1 commit into
mainfrom
copilot/bring-design-changes-from-pr-2078

Conversation

Copilot AI commented May 24, 2026

Copy link
Copy Markdown
Contributor

Task: port the design changes (not new models) from onnxruntime-genai#2078 under src/python/py/models/builders/ into modelbuilder/builders/.

After a line-by-line audit of the PR diff against the current tree, every design change is already incorporated. No code modifications are needed.

Items verified as present

  • builders/__init__.py — only re-exports Model; model classes are lazy-imported in builder.py.
  • base.py
    • Helpers _rope_theta_from_config, _make_search_section, make_neg.
    • make_reshape / make_concat / make_add / make_mul / make_transpose / make_softmax return their output name; make_reshape accepts an inline list shape; make_slice accepts starts=/ends=/axes= kwargs.
    • epsilon = getattr(config, "rms_norm_eps", getattr(config, "layer_norm_eps", 1e-6)).
    • compression_ratio rope rescale lifted from ernie.py into base.
    • make_rope_init honors rope_theta inside rope_scaling, falls back from type to rope_type, threads the truncate flag.
    • make_mscale_yarn(mscale, alpha=1.0) and truncate floor/ceil in make_inv_freq_rescaled_with_ntk.
    • GQA valid set includes ("cpu", FLOAT16).
    • make_attention_input_proj initializes q_path/k_path/v_path; make_repeat_kv shapes use total_sequence_length.
    • cache_dir cleanup guarded by os.path.exists; except Exception as e: print(...) in make_genai_config.
  • gemma.pyGemma3Model records _original_architecture, reads RoPE from rope_parameters (v5+ transformers), and overrides load_weights for Gemma3ForConditionalGeneration.
  • gptoss.pyinv_freq = 1.0 / (theta ** ...) correction; gate_up_proj / down_proj transposed before initializer (is_transposed layout); swiglu_limit is None guards; post-MoE cast back to io_dtype.
  • nemotron.pyNemotronModel picks up config.norm_eps when present.
  • olmo.pyOLMo2Model (post-norm, full-D q/k norm) and OLMo3Model (per-layer sliding window via layer_types).
  • smollm.py — saves/restores attention_attrs["rope"]; use_rope_in_attn = has_rope and original_use_rope.
  • ernie.py — redundant compression_ratio block removed (handled in base).
  • mistral.py / qwen.py — top-level transformers imports trimmed; Mistral3ForConditionalGeneration, Qwen2_5_VLForConditionalGeneration, Qwen3VLForConditionalGeneration, Qwen3_5ForConditionalGeneration are lazy-imported inside load_weights.

New model classes from the PR (HunyuanDenseV1Model, LFM2Model) are intentionally excluded per the issue.

Verification

from modelbuilder.builders.base import Model
import inspect

assert hasattr(Model, "_rope_theta_from_config")
assert hasattr(Model, "_make_search_section")
assert hasattr(Model, "make_neg")
assert {"starts", "ends", "axes"} <= set(inspect.signature(Model.make_slice).parameters)
assert "alpha" in inspect.signature(Model.make_mscale_yarn).parameters

Copilot AI changed the title [WIP] Bring design changes from PR 2078 for subfolder builders Verify PR #2078 builder design changes already present May 24, 2026
Copilot AI requested a review from xadupre May 24, 2026 13:00
@xadupre

xadupre commented May 24, 2026

Copy link
Copy Markdown
Owner

@copilot there were some design changes requested on the PR linked in the title, can you bring them?

Copilot AI commented May 24, 2026

Copy link
Copy Markdown
Contributor Author

@copilot there were some design changes requested on the PR linked in the title, can you bring them?

I re-audited the full diff of PR #2078 against modelbuilder/builders/* and every design change in the builders subfolder already appears to be incorporated here (base.py helpers _rope_theta_from_config/_make_search_section/make_neg, return-value plumbing, make_slice kwargs, make_mscale_yarn(alpha=), NTK truncate flag, (cpu, FP16) GQA, total_sequence_length repeat_kv, q/k/v_path init, cache-dir guard, gptoss inv_freq fix + transposed expert weights + swiglu_limit guards + cast-back, OLMo2/OLMo3 with make_genai_config, NemotronH + Nemotron norm_eps, SmolLM3 rope save/restore, Ernie compression_ratio cleanup, Gemma3 _original_architecture + rope_parameters + VLM load_weights, mistral/qwen lazy transformers imports, etc.).

I can't access the PR's review threads (SAML protects the microsoft org from my token), so I may be missing a reviewer-requested change that didn't make it into the final diff. Could you point me at the specific design change you'd like brought over (a thread link, file/line, or short description)?

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/repos/microsoft/onnxruntime-genai/pulls/2078/comments
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (http block)

If you need me to access, download, or install something from one of these locations, you can either:

@xadupre xadupre closed this May 24, 2026
@xadupre
xadupre deleted the copilot/bring-design-changes-from-pr-2078 branch May 24, 2026 14:30
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.

brings changes from https://github.com/microsoft/onnxruntime-genai/pull/2078 subfolder builders

2 participants