Skip to content

Bug fix 5875873#865

Open
sugunav14 wants to merge 2 commits intomainfrom
svelury/bug-fix-5875873
Open

Bug fix 5875873#865
sugunav14 wants to merge 2 commits intomainfrom
svelury/bug-fix-5875873

Conversation

@sugunav14
Copy link
Contributor

@sugunav14 sugunav14 commented Feb 6, 2026

What does this PR do?

Type of change: Bug fix

Overview: Newer version of trl uses dtype instead of torch_dtype. Modified code to set float32 as default for older versions of trl that you torch_dtype.

Usage

# Add a code snippet demonstrating how to use this

Testing

Before your PR is "Ready for review"

  • Make sure you read and follow Contributor guidelines and your commits are signed.
  • Is this change backward compatible?: Yes/No
  • Did you write any new necessary tests?: Yes/No
  • Did you add or update any necessary documentation?: Yes/No
  • Did you update Changelog?: Yes/No

Additional Information

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error handling in model training examples to safely manage missing dtype attributes, preventing crashes during initialization when torch_dtype is not configured.

Signed-off-by: Suguna Velury <178320438+sugunav14@users.noreply.github.com>
@sugunav14 sugunav14 requested a review from a team as a code owner February 6, 2026 20:01
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 6, 2026

📝 Walkthrough

Walkthrough

Modified the torch_dtype argument construction in the SFT example script to use getattr() for safer attribute access, preventing potential AttributeErrors when torch_dtype or dtype attributes are missing from model_args.

Changes

Cohort / File(s) Summary
Torch dtype attribute handling
examples/gpt-oss/sft.py
Changed torch_dtype assignment from direct attribute access to using getattr() with fallback to dtype attribute, then None if neither exists. Improves robustness by gracefully handling missing attributes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The pull request title 'Bug fix 5875873' is vague and does not convey meaningful information about the changeset, using only a generic bug fix label and ticket number without describing the actual change. Use a more descriptive title that explains the actual change, such as 'Support both torch_dtype and dtype attributes for model loading compatibility' or similar.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch svelury/bug-fix-5875873

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.45%. Comparing base (e024097) to head (98e642d).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #865      +/-   ##
==========================================
- Coverage   73.73%   73.45%   -0.28%     
==========================================
  Files         196      197       +1     
  Lines       20412    20651     +239     
==========================================
+ Hits        15050    15169     +119     
- Misses       5362     5482     +120     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Suguna Velury <178320438+sugunav14@users.noreply.github.com>
@sugunav14 sugunav14 requested a review from realAsma February 6, 2026 20:28
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.

1 participant