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

Fix callbacks for pre-trained model, use legacy optimizer #214

Merged
merged 3 commits into from
Jan 18, 2023

Conversation

bfhealy
Copy link
Collaborator

@bfhealy bfhealy commented Jan 18, 2023

This PR fixes an error that was raised when running additional training on pre-trained models. The callbacks metadata for the pre-trained model is not saved and so cannot be loaded in like the rest of the model components. Thus, there is now a separate set_callbacks method in nn.py which is called when loading in a pre-trained model.

Additionally, this PR adds compatibility with the latest tensorflow-macos (2.11), which raises an error when accessing tf.keras.optimizers.Adam. The replacement, tf.keras.optimizers.legacy.Adam, is also included in the previous tensorflow-macos version (2.10), so this change will not break scope on environments set up in September 2022 and onward.

(On the topic of tensorflow versions and M1 Macs, see also the updates in #125 pertaining to GPU support.)

Copy link
Collaborator

@mcoughlin mcoughlin left a comment

Choose a reason for hiding this comment

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

Nice! LGTM!

@bfhealy
Copy link
Collaborator Author

bfhealy commented Jan 18, 2023

@mcoughlin I forgot that for machines other than M1 Macs, we require tensorflow < 2.6, explaining the test failure. I could either remove this limit on the tf version or set up a version-checking if-else in the nn.py code.

@mcoughlin
Copy link
Collaborator

@bfhealy I think removing the limit is fine assuming everything works?

@bfhealy
Copy link
Collaborator Author

bfhealy commented Jan 18, 2023

@mcoughlin Sounds good, I'll remove the limit and see how the tests run.

@bfhealy bfhealy merged commit f9da8bc into ZwickyTransientFacility:main Jan 18, 2023
@bfhealy bfhealy deleted the callbacks branch January 18, 2023 20:59
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