Skip to content

Added dice loss - #224

Closed
ayyucedemirbas wants to merge 2 commits into
ml-explore:mainfrom
ayyucedemirbas:main
Closed

ayyucedemirbas wants to merge 2 commits into
ml-explore:mainfrom
ayyucedemirbas:main

Conversation

@ayyucedemirbas

Copy link
Copy Markdown

Proposed changes

Please include a description of the problem or feature this PR is addressing. If there is a corresponding issue, include the issue #.

Checklist

Put an x in the boxes that apply.

  • I have read the CONTRIBUTING document
  • I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the necessary documentation (if needed)

@awni awni 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 the contribution!!

This loss seems a little niche to add it to mlx.nn.losses. I'm open to discussion on it, but unless there is a good case / strong need, I would table this addition for now and bring it back if it's useful.

@ayyucedemirbas

Copy link
Copy Markdown
Author

Thank you so much for your time and quick response. It can be useful for image segmentation tasks. But the final decision is yours 🙂 I would like to add other things (losses, layers, activation functions, etc.), but I don't know what would be useful. I think a list of things to add could be helpful for the contributors.

@awni

awni commented Dec 21, 2023

Copy link
Copy Markdown
Member

Great. Let's close this for now. And we can revisit in the futre.

A list of open items that would make for good contributions is a nice idea :). In the meantime, a good place to start is to check out the issues an look for the label enhancements. Feel free to post on the issue if you have questions about the feature.

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