Skip to content

Add get_config method to Loss #586

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

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Add get_config method to Loss #586

wants to merge 7 commits into from

Conversation

mberr
Copy link
Member

@mberr mberr commented Sep 6, 2021

This PR adds a get_config method to Loss and its subclasses. The method is inspired by https://keras.io/api/layers/base_layer/#getconfig-method and can be used to obtain all parameters necessary to re-instantiate an equivalent instance. It prefers "simple" forms, e.g., converts the reduction method of margin activation to string representation.

This method can be used in the future for logging hyperparameters, or creating JSONified configurations.

There is also a test which checks whether the re-instantiated loss instance produces the same loss values for a random batch of scores.

Alternative

Instead of making this a class method, we could also extend the class resolver to also provide the "inverse way" of extracting the configuration from an instance.

def get_config(self) -> MutableMapping[str, Any]: # noqa: D102
config = super().get_config()
config["margin"] = self.margin
config["margin_activation"] = margin_activation_resolver.normalize_inst(self.margin_activation)
Copy link
Member

Choose a reason for hiding this comment

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

what if the activation has kwargs? should this be recursive?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this should also consider kwargs 🤔 Since activations usually are PyTorch objects, this is may be problematic - so maybe we need the second alternative described above, where the resolver gets the get_config method?

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