Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: logger is unbound error #308

Merged
merged 3 commits into from
Aug 21, 2024
Merged

fix: logger is unbound error #308

merged 3 commits into from
Aug 21, 2024

Conversation

HarikrishnanBalagopal
Copy link
Contributor

Description of the change

Fixes #307

Related issue number

#307

How to verify the PR

python3 -m tuning.sft_trainer --output_dir foo --use_flash_attn false --bnb_qlora true

Was the PR tested

  • I have added >=1 unit test(s) for every new method I have added.
  • I have ensured all unit tests pass

@HarikrishnanBalagopal
fix: logger is unbound error
6b26df3 
Signed-off-by: Harikrishnan Balagopal <harikrishmenon@gmail.com>
@HarikrishnanBalagopal HarikrishnanBalagopal marked this pull request as draft August 19, 2024 05:32
@HarikrishnanBalagopal
fixup! fix: logger is unbound error
2ba1a7d 
Signed-off-by: Harikrishnan Balagopal <harikrishmenon@gmail.com>
@ashokponkumar ashokponkumar marked this pull request as ready for review August 20, 2024 14:53
@Ssukriti Ssukriti requested review from aluu317 and removed request for alex-jw-brooks, Ssukriti and anhuong August 20, 2024 15:55
@Ssukriti
Copy link
Collaborator

@aluu317 could you review this as you recently went over logging code?

@Ssukriti Ssukriti requested a review from anhuong August 20, 2024 18:09
anhuong
anhuong approved these changes Aug 21, 2024
View reviewed changes
Copy link
Collaborator

@anhuong anhuong left a comment

Choose a reason for hiding this comment

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

Need to initialize logger so it can be used in the exception if parse_argumetns fails

@anhuong anhuong merged commit daca551 into foundation-model-stack:main Aug 21, 2024
7 checks passed
@ashokponkumar
Copy link
Collaborator

Thanks @anhuong !

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

@anhuong anhuong anhuong approved these changes

@aluu317 aluu317 Awaiting requested review from aluu317 aluu317 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug: UnboundLocalError, logger is not defined
4 participants
@HarikrishnanBalagopal @Ssukriti @ashokponkumar @anhuong