Skip to content

Add missing math and sys imports in llava_trainer.py#505

Open
Mr-Neutr0n wants to merge 1 commit into
LLaVA-VL:mainfrom
Mr-Neutr0n:fix/missing-math-sys-imports
Open

Add missing math and sys imports in llava_trainer.py#505
Mr-Neutr0n wants to merge 1 commit into
LLaVA-VL:mainfrom
Mr-Neutr0n:fix/missing-math-sys-imports

Conversation

@Mr-Neutr0n

Copy link
Copy Markdown

Summary

  • Adds import math and import sys to llava/train/llava_trainer.py
  • _inner_training_loop uses math.ceil() and sys.maxsize but neither module was imported
  • Training crashes with NameError: name 'math' is not defined

Related Issue

Fixes #504

Test plan

  • Run training command to verify no more NameError
  • Verify training proceeds normally

The _inner_training_loop method uses math.ceil() and sys.maxsize but
neither module was imported, causing NameError during training.

Fixes LLaVA-VL#504
@Mr-Neutr0n

Copy link
Copy Markdown
Author

hey, just checking in. this adds missing math and sys imports in llava_trainer.py that cause NameError at runtime. one-liner fix

@Mr-Neutr0n

Copy link
Copy Markdown
Author

Friendly bump! Let me know if there's anything I should update or improve to help move this forward.

@Mr-Neutr0n

Copy link
Copy Markdown
Author

This is still missing a reviewer. The change is straightforward -- adding the math and sys imports that llava_trainer.py already uses (for example, math.isnan checks on loss values). Happy to rebase if needed.

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.

Training crash on main: NameError: name 'math' is not defined in llava_trainer.py

1 participant