-
Notifications
You must be signed in to change notification settings - Fork 0
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
Don't fail if optim name not found. #54
Conversation
src/neptune_fastai/impl/__init__.py
Outdated
@@ -154,7 +154,9 @@ def _batch_size(self) -> int: | |||
|
|||
@property | |||
def _optimizer_name(self) -> Optional[str]: | |||
return self.opt_func.__name__ | |||
optim_name = getattr(self.opt_func, "__name__", "NA") | |||
warnings.warn("NeptuneCallback: Couldn't retrieve the optimizer name, so it will logged as 'NA'.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"NeptuneCallback: Couldn't retrieve the optimizer name, so it will be logged as 'NA'."
or should it be 'N/A'? not sure which option will be consistent with other instances of this in Neptune, but usually this abbreviation includes a slash :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we not skip logging this field instead of logging it as NA?
The warning can also be updated to
"NeptuneCallback: Couldn't retrieve the optimizer name, so it will not be logged.
You can set the optimizer name by passing it to the __name__
attribute:
>>> optimizer.__name__ = "NAME"
"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason, I thought of logging something like NA
was that if later the user has a script which fetches values from run then this would fail because it doesn't exist in this case (So changing the structure would be BC-breaking). Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense (y)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will update the message to
"NeptuneCallback: Couldn't retrieve the optimizer name, so it will be logged as "N/A". You can set the optimizer name by assigning it to the __name__ attribute. Eg. >>> optimizer.__name__ = 'NAME'
Thanks!
src/neptune_fastai/impl/__init__.py
Outdated
@@ -154,7 +154,9 @@ def _batch_size(self) -> int: | |||
|
|||
@property | |||
def _optimizer_name(self) -> Optional[str]: | |||
return self.opt_func.__name__ | |||
optim_name = getattr(self.opt_func, "__name__", "NA") | |||
warnings.warn("NeptuneCallback: Couldn't retrieve the optimizer name, so it will logged as 'NA'.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we not skip logging this field instead of logging it as NA?
The warning can also be updated to
"NeptuneCallback: Couldn't retrieve the optimizer name, so it will not be logged.
You can set the optimizer name by passing it to the __name__
attribute:
>>> optimizer.__name__ = "NAME"
"
…-ai/neptune-fastai into fix/optim_name_not_found
Ref: neptune-ai/neptune-client#1406