Skip to content

add more loss function options - #159

Merged
lewtun merged 7 commits into
huggingface:mainfrom
PhilipMay:add_more_loss_options
Nov 8, 2022
Merged

add more loss function options#159
lewtun merged 7 commits into
huggingface:mainfrom
PhilipMay:add_more_loss_options

Conversation

@PhilipMay

@PhilipMay PhilipMay commented Nov 7, 2022

Copy link
Copy Markdown
Contributor

Add more options to the non default loss functions.
This is distance_metric and margin.
The default values are selected in a way that this is no breaking change.

Todo

  • add docstrings

@PhilipMay PhilipMay changed the title Add more loss function options [WIP] Add more loss function options Nov 7, 2022
@PhilipMay PhilipMay changed the title [WIP] Add more loss function options Add more loss function options Nov 7, 2022
@PhilipMay

PhilipMay commented Nov 7, 2022

Copy link
Copy Markdown
Contributor Author

CI is green, this is done and can be merged from my point of view.

@lewtun lewtun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for adding these additional args @PhilipMay 🤗 !

I think at some point we'll need to refactor this out to a dedicated class for the training args (like the transformers.Trainer), but happy to have this included for now :)

@lewtun
lewtun merged commit 641fbfc into huggingface:main Nov 8, 2022
@PhilipMay

Copy link
Copy Markdown
Contributor Author

Thanks for adding these additional args @PhilipMay 🤗 !

I think at some point we'll need to refactor this out to a dedicated class for the training args (like the transformers.Trainer), but happy to have this included for now :)

Should we collect this "future ideas" anywhere?

@PhilipMay
PhilipMay deleted the add_more_loss_options branch November 8, 2022 14:32
@PhilipMay PhilipMay changed the title Add more loss function options add more loss function options Nov 14, 2022
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