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

Update learning_rate type to float in learning_rate_scheduler's logs #649

Conversation

siyavash
Copy link
Contributor

@siyavash siyavash commented Aug 1, 2023

Description:

This merge request updates the learning_rate type to float in the learning_rate_scheduler's logs for the following reasons:

Reason 1:
Currently, when the learning_rate is not converted to floats, the history logs show that we are using the same parameter value, resulting in a display like this:

'learning_rate': [Parameter containing:
tensor(3.9063e-04),
Parameter containing:
tensor(3.9063e-04),
Parameter containing:
tensor(3.9063e-04),
...
Parameter containing:
tensor(3.9063e-04)]

This issue occurs we are keeping the parameter type throughout the epochs. Consequently, the parameter type remains the same, but its value changes dynamically as the training progresses. As a result, only the last value is visible, and we lose the history of the learning rate.

Reason 2:
The current parameter type is not serializable, which can cause issues when using certain callbacks, such as WandbMetricsLogger callback, which requires serializable data. By updating the learning_rate type to float, we resolve this serialization issue and ensure smooth integration with such callbacks.

Proposed Solution:
By converting the learning_rate to floats in the logs, we preserve the entire history of the learning rate, making it easier to track and analyze changes. Additionally, we address the serialization problem, enabling seamless integration with callbacks that require serializable data.

Impact and Testing:
These changes do not impact the functionality of the learning rate scheduler itself. The update simply ensures better logging and serialization. I have added a unit test to ensure that we have the learning_rate in history as expected.

Copy link
Contributor

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

LGTM -- indeed, anything that makes it into the logs should be a Python type.

@fchollet fchollet merged commit c466112 into keras-team:main Aug 1, 2023
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