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

DICE loss #296

Closed
innat opened this issue Apr 12, 2022 · 16 comments
Closed

DICE loss #296

innat opened this issue Apr 12, 2022 · 16 comments

Comments

@innat
Copy link
Contributor

innat commented Apr 12, 2022

It is one of the most used loss functions in semantic segmentation tasks. Unfortunately still not available in keras.loss.*. It has been asked many times, #3611, #13085, #9395, #10890.

(Not sure, if it's fit here or keras.loss.*)

@bhack
Copy link
Contributor

bhack commented Apr 12, 2022

@innat
Copy link
Contributor Author

innat commented Apr 12, 2022

Okayyy, someone pushed. I think this function should be available here. It's widely used.
cc. @gamenerd457

@gamenerd457
Copy link

Interested

@bhack
Copy link
Contributor

bhack commented Apr 13, 2022

@LukeWood If you want this please assign it to @gamenerd457 to port his PR here.

@innat
Copy link
Contributor Author

innat commented Apr 18, 2022

@LukeWood @qlzh727
Could u please give some feedback on this and also #341?

@qlzh727 qlzh727 self-assigned this Apr 18, 2022
@qlzh727
Copy link
Member

qlzh727 commented Apr 18, 2022

Yea, I think we can host this here (adding this to keras repo might need more extended API review and requirement). We should put this to keras_cv/losses folder.

@bhack
Copy link
Contributor

bhack commented Apr 18, 2022

@gamenerd457 Green light for a PR here.

@LukeWood
Copy link
Contributor

yeah seems like a great fit!

@gamenerd457
Copy link

So should I make a pr here

@LukeWood
Copy link
Contributor

So should I make a pr here

If the loss easily confirms to the Keras loss API (y_true, y_pred) then yes!

@bhack
Copy link
Contributor

bhack commented Apr 19, 2022

@LukeWood The intention was to port/refactoring @gamenerd457's Addons PR https://github.com/tensorflow/addons/pull/2558/files

@innat
Copy link
Contributor Author

innat commented Apr 20, 2022

@gamenerd457 (cc. @bhack @LukeWood @qlzh727 )

If the loss easily confirms to the Keras loss API (y_true, y_pred) then yes!

Some refactoring may be needed, (i.e. subclassing the tf.keras.losses.Loss class). I think you can start working on it, either you can move PR from tf-addons or create a new one here. The implementation confirms the keras loss API.

Also please follow the implementation details from segmentation_models/losses.py. This reference implementation offers some useful initial parameter (i.e per_image, class_weights etc), which I found very useful in practice.

def dice(y_true, y_pred, ...):
    return loss

class Dice(keras.losses.Loss):
    def __init__(self, 
                beta=1, 
                class_weights=None, 
                class_indexes=None, 
                per_image=False, smooth=SMOOTH, name='Dice'):
        super().__init__(name=name)
      
    def call(self, y_true, y_pred):
        return dice(...)

@LukeWood
Copy link
Contributor

What are some use cases for per_image?

Also note that in keras we won't support class_weights; instead, we will support sample_weights in call.

@innat
Copy link
Contributor Author

innat commented Apr 20, 2022

From HERE.

per_image: If True loss is calculated for each image in batch and then averaged,
else loss is calculated for the whole batch.

Enabling it to True provides a performance boost depending on the task.

@LukeWood
Copy link
Contributor

Cool, we will need to include this in the docstring. Seems like a match!

@innat
Copy link
Contributor Author

innat commented Nov 24, 2022

@DavidLandup0

Here are some info that might be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants