Skip to content

fix: Use trust_remote_code_for_model() in all from_pretrained calls#229

Draft
kendrickb-nvidia wants to merge 1 commit intomainfrom
kendrickb-nvidia/fix-trust-remote-code
Draft

fix: Use trust_remote_code_for_model() in all from_pretrained calls#229
kendrickb-nvidia wants to merge 1 commit intomainfrom
kendrickb-nvidia/fix-trust-remote-code

Conversation

@kendrickb-nvidia
Copy link
Copy Markdown
Collaborator

@kendrickb-nvidia kendrickb-nvidia commented Mar 13, 2026

Summary

  • Some AutoConfig, AutoModelForCausalLM, and AutoTokenizer from_pretrained calls were missing trust_remote_code=True for nvidia/ models (e.g. Nemotron), causing ValueError when loading models with custom code
  • Consolidates the check into a single trust_remote_code_for_model() in llm/utils.py, called by all 8 ModelMetadata subclasses, populate_derived_fields, LLMPromptConfig.from_tokenizer, and HuggingFaceBackend
  • Removes the redundant TrainingBackend._trust_remote_code_for_model() method

Test plan

  • Verified nvidia/NVIDIA-Nemotron-3-Nano-30B-A3B-BF16 loads successfully through the SDK SafeSynthesizer pipeline
  • Unit tests pass (make test)

Other notes

Created #231 to followup and make modifying this behavior use configurable.

Made with Cursor

…dia/ models

AutoConfig, AutoModelForCausalLM, and AutoTokenizer.from_pretrained calls
were missing trust_remote_code=True for nvidia/ models (e.g. Nemotron),
causing ValueError when loading models with custom code. Consolidates the
check into a single trust_remote_code_for_model() in llm/utils.py used by
both ModelMetadata subclasses and HuggingFaceBackend.

Signed-off-by: Kendrick Boyd <kendrickb@nvidia.com>
Made-with: Cursor
@kendrickb-nvidia kendrickb-nvidia requested a review from a team as a code owner March 13, 2026 19:23
@kendrickb-nvidia kendrickb-nvidia changed the title fix: pass trust_remote_code through all from_pretrained calls for nvidia/ models fix: Use trust_remote_code_for_model() in all from_pretrained calls Mar 13, 2026
@kendrickb-nvidia
Copy link
Copy Markdown
Collaborator Author

Also more from_pretrained() like calls during generation that also need to pick up this trust remote code logic. Need to follow up on those before merging.

@kendrickb-nvidia kendrickb-nvidia marked this pull request as draft March 20, 2026 23:37
@binaryaaron
Copy link
Copy Markdown
Collaborator

good consolidation -- the populate_derived_fields fix is the important one and the duplicate removal from TrainingBackend is clean.

one thing: the utils.py change narrows the type from str | Path to str and drops the str() coercion. huggingface_backend.py passes self.params.training.pretrained_model which could be a Path from config and Path doesn't have .startswith().

this will conflict with #286 and #287 since both touch every subclass init in metadata.py.

I'd plan to land #286 first and then either #287 or this. the rebases should be pretty straightforward.

longer term the repeated AutoTokenizer.from_pretrained(..., trust_remote_code=...)/AutoConfig.from_pretrained(...) pattern in every subclass is asking to be a base-class helper. #231 (making it configurable) would be a good time to do that.

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.

2 participants