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

Added new flexible loss named GeneralLoss #1647

Closed
wants to merge 7 commits into from

Conversation

ashutosh1919
Copy link
Contributor

@ashutosh1919 ashutosh1919 commented Apr 11, 2020

fixes #1646.
@gabrieldemarmiesse , @pavithrasv and @mihaimaruseac, please review.

@WindQAQ WindQAQ self-assigned this Apr 15, 2020
Copy link
Member

@WindQAQ WindQAQ left a comment

Choose a reason for hiding this comment

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

Hi @ashutosh1919 , thanks for the PR. Because this PR is somewhat large, let's improve it step by step :-)

For testing, please follow other tests in loss module.
https://github.com/tensorflow/addons/blob/master/tensorflow_addons/losses/tests/kappa_loss_test.py

tensorflow_addons/losses/general_loss.py Show resolved Hide resolved
tensorflow_addons/losses/general_loss.py Outdated Show resolved Hide resolved
tensorflow_addons/losses/general_loss.py Show resolved Hide resolved
tensorflow_addons/losses/tests/general_loss_test.py Outdated Show resolved Hide resolved
tensorflow_addons/losses/tests/general_loss_test.py Outdated Show resolved Hide resolved
@gabrieldemarmiesse
Copy link
Member

gabrieldemarmiesse commented Apr 17, 2020

As a side note, maybe we should reconsider the name "GeneralLoss"? It may be confusing for users. They may think it's some kind of abstract base class.

We also changed the CI quite a bit recently, so I'm merging master to make sure everything works. Please do git pull before adding new commits.

@googlebot

This comment has been minimized.

@gabrieldemarmiesse

This comment has been minimized.

@googlebot

This comment has been minimized.

@ashutosh1919
Copy link
Contributor Author

@gabrieldemarmiesse , Yes you are right. We should think about better name here. GeneralLoss is quite confusing.
So, @gabrieldemarmiesse and @WindQAQ , please suggest name if you have any in your mind. I will also search for it.

@ashutosh1919
Copy link
Contributor Author

@gabrieldemarmiesse , @WindQAQ - How about the name UniversalLoss (universal_loss) ?

@AakashKumarNain
Copy link
Member

Not a good one IMO

@gabrieldemarmiesse
Copy link
Member

gabrieldemarmiesse commented Apr 17, 2020

There are only two hard things in Computer Science: cache invalidation and naming things

  • A famous dude

Cache invalidation is easier IMO

@ashutosh1919
Copy link
Contributor Author

Some suggestions for name:

  • generalized_gaussian_loss (from research paper, it is noted that the behavior of this loss is similar as Generalized Gaussian Distribution.)
  • cauchy_gaussian_loss (It is also mentioned in the paper that this loss covers the distribution which is intersection of cauchy and gaussian distributions.)
  • general_robust_loss (general_loss may confuse developers but this one will not. And btw, this loss is one among the robust_loss implemented by google research.)

@ashutosh1919
Copy link
Contributor Author

@gabrieldemarmiesse , Any update on this PR? Thanks!

@bhack
Copy link
Contributor

bhack commented May 13, 2020

gar_loss (General and Adaptive Robust Loss)

@ashutosh1919
Copy link
Contributor Author

gar_loss (General and Adaptive Robust Loss)

@bhack,
Don't you think adaptive_loss is different than general_loss as per the code here.

@ashutosh1919
Copy link
Contributor Author

@gabrieldemarmiesse , @bhack , @AakashKumarNain - Any suggestion for name to go ahead with this PR?

@bhack
Copy link
Contributor

bhack commented May 13, 2021

It don't know what to do exactly with all these Docs repos.
We have focal also in https://github.com/keras-team/keras-cv

@ashutosh1919
Copy link
Contributor Author

@bhack , I think you misunderstood this PR with #1732 . That PR was focusing on doc tutorials for Focal loss & Contrastive loss.

This PR adds a new loss General loss.

@bhack
Copy link
Contributor

bhack commented May 13, 2021

Yes I was fooled by github notifications

@seanpmorgan
Copy link
Member

Thank you for your contribution. We sincerely apologize for any delay in reviewing, but TensorFlow Addons is transitioning to a minimal maintenance and release mode. New features will not be added to this repository. For more information, please see our public messaging on this decision:
TensorFlow Addons Wind Down

Please consider sending feature requests / contributions to other repositories in the TF community with a similar charters to TFA:
Keras
Keras-CV
Keras-NLP

@seanpmorgan seanpmorgan closed this Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need of flexible loss function GeneralLoss which can change behavior based on params
7 participants