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.

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

Gb/optm state #242

Merged
merged 6 commits into from
Nov 14, 2024
Merged

Gb/optm state #242

merged 6 commits into from
Nov 14, 2024

Conversation

grantbuster
Copy link
Member

No description provided.

Copy link
Collaborator

@bnb32 bnb32 left a comment

Choose a reason for hiding this comment

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

Looks good. Any interesting results yet?

@grantbuster
Copy link
Member Author

Looks good. Any interesting results yet?

not really, i think we'll need more training time to see if there's a trajectory. Some layers are definitely gaining momentum on gradients but not sure if they'll converge.

f'received error: "{e}"')
logger.warning(msg)
warn(msg)
ncfile.setncattr(attr_name, json.dumps(attr_value))
Copy link
Member Author

Choose a reason for hiding this comment

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

@bnb32 fyi i was getting a weird error in production runs where the model meta data had objects that were not compatible with NetCDF. I added some extra error handling here.

Copy link
Collaborator

@bnb32 bnb32 Nov 13, 2024

Choose a reason for hiding this comment

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

@grantbuster ok cool. I removed some of the default serializing I had before bc it was casting too many things to strings, guess I removed too many. you should use the safe_serialize function:

def safe_serialize(obj, **kwargs):
instead of json.dumps here.

Copy link
Member Author

Choose a reason for hiding this comment

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

smart, done!

…ta as attrs, needs to be serialized another way, added error handling here
@grantbuster grantbuster merged commit 9c34e65 into main Nov 14, 2024
12 checks passed
@grantbuster grantbuster deleted the gb/optm_state branch November 14, 2024 15:59
@grantbuster grantbuster restored the gb/optm_state branch November 14, 2024 15:59
@grantbuster grantbuster deleted the gb/optm_state branch November 14, 2024 15:59
github-actions bot pushed a commit that referenced this pull request Nov 14, 2024
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