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 formatting of loss spike & high loss error messages #1498

Merged
merged 88 commits into from
Aug 29, 2024
Merged

Fix formatting of loss spike & high loss error messages #1498

merged 88 commits into from
Aug 29, 2024

Conversation

joyce-chen-uni
Copy link
Contributor

@joyce-chen-uni joyce-chen-uni commented Aug 29, 2024 •
edited
Loading

Fix weird formatting in error messages (only applies to low code runs):
Screenshot 2024-08-29 at 1 30 03 PM
Screenshot 2024-08-29 at 3 27 49 PM

@joyce-chen-uni
Kill run on loss spike callback + init
8e216a7
@joyce-chen-uni
Import
a1a294b
@joyce-chen-uni
Add logger as arg
601e64a
@joyce-chen-uni
Attribute
95520c1
@joyce-chen-uni
Logging for debugging
783d02d
@joyce-chen-uni
Only check potential loss spike if loss window sufficient size
0b2eba4
@joyce-chen-uni
Need to track whether previous step was a potential spike
e78e9b4
@joyce-chen-uni
Bug fix loss window append + simplify
18256a5
@joyce-chen-uni
Bug fix loss window append + simplify
d300726
@joyce-chen-uni
add spike yaml temporarily
44ad527
@joyce-chen-uni
Low parameters for testing
d0523ac
@joyce-chen-uni
Delete logger
eb5fdba
@joyce-chen-uni
Custom error for loss spike
ad5c5e9
@joyce-chen-uni
Detailed error
2a135d6
@joyce-chen-uni
Reduce logging
06b958e
@joyce-chen-uni
Delete yaml
c9cfad2
@joyce-chen-uni
add a condition for stopping if generally very high losses
7b4d31e
@joyce-chen-uni
Test logging
12e878e
@joyce-chen-uni
Just test metadata logging
ee5ea13
@joyce-chen-uni
Move window slide to the end so that cur loss not factored into runni…
f24ef7e 
…ng loss
@joyce-chen-uni
log/fail depending on wall clock
6779b3d
@joyce-chen-uni
No loggin
a496028
@joyce-chen-uni
Try to ensure checkpoint saved before killing run
ad88a43
@joyce-chen-uni
test
8eb7824
@joyce-chen-uni
Log metadata dict
20261bd
@joyce-chen-uni
testing
a76996f
@joyce-chen-uni
Log all potential spikes to run metadata
e646b2a
@joyce-chen-uni
Test logging
ebd8ff5
@joyce-chen-uni
log to be sure
53f4117
@joyce-chen-uni
Final
decf9c5
@joyce-chen-uni
Logging for window size setting check
8ec812c
@joyce-chen-uni
Add back window_size and loss_cap as optl params
93b0fe8
@joyce-chen-uni
Test that error is not raised when log_only set to true
2214175
@joyce-chen-uni
Update docstring
57800e6
@joyce-chen-uni
Round fractional window size
c59e5be
@joyce-chen-uni
Window size and loss cap not specifiable
1aeb46f
@joyce-chen-uni
Adjust tests no window size / loss cap inputs
1b5a757
@joyce-chen-uni
init vals for window size, loss window, loss cap
e3c00b3
@joyce-chen-uni
decompose spike/high loss handling
04d20fa
@joyce-chen-uni
decompose logging and rename helpers
4658a8f
@joyce-chen-uni
Test error raised/not raised depending on log_only
55dd5bb
@joyce-chen-uni
Cleanup window frac
5b044fa
@joyce-chen-uni
Clean up fit_start & callback rank 0 only
95bed00
@joyce-chen-uni
Try moving self._enabled setting
50a6b5e
@joyce-chen-uni
Private consts
9f37281
@joyce-chen-uni
Add absolute loss cap and loss window as arguments
acd46f4
@joyce-chen-uni
Take out optional type
782316b
@joyce-chen-uni
Calculate the loss window once hit min loss window size
187c861
@joyce-chen-uni
Test window size setting
47a461c
@joyce-chen-uni
Keep old deque
fee774b
@joyce-chen-uni
Cleanup
3e2ecc6
@joyce-chen-uni
Remove running loss avg log
4005a5c
@joyce-chen-uni
Merge branch 'mosaicml:main' into main
fd7ece5
@joyce-chen-uni
Fix formatting of error messages
9f4bdb9
@joyce-chen-uni joyce-chen-uni requested a review from a team as a code owner August 29, 2024 19:40
jjanezhang
jjanezhang approved these changes Aug 29, 2024
View reviewed changes
Copy link
Contributor

@jjanezhang jjanezhang left a comment

Choose a reason for hiding this comment

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

lgtm

dakinggg
dakinggg approved these changes Aug 29, 2024
View reviewed changes
Copy link
Collaborator

@dakinggg dakinggg left a comment

Choose a reason for hiding this comment

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

Please add a PR description. This change of course looks pretty harmless, but also I'm not sure why its being made.

@joyce-chen-uni joyce-chen-uni merged commit 07bec5a into mosaicml:main Aug 29, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jjanezhang jjanezhang jjanezhang approved these changes

@dakinggg dakinggg dakinggg approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@joyce-chen-uni @jjanezhang @dakinggg