-
-
Notifications
You must be signed in to change notification settings - Fork 650
Add SSIM for 3D tensors #3345
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
Add SSIM for 3D tensors #3345
Conversation
Thanks for the PR @lrlunin! Is it possible to extend exisitng SSIM class to cover 3D case instead of creating SSIM3D class. I have an impression that SSIM3D is mostly a copy of SSIM...
What do you think? |
Yeah, this would work. I just wasn't sure if you agree to change the original class. I'll rework this version towards your tipp and mention you after it's done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates.
I left few comments.
Co-authored-by: vfdev <vfdev.5@gmail.com>
@lrlunin please fix mypy errors:
Also please rebase the PR on top of the master branch. |
Should we add |
Ultimately, in another PR I would like to remove |
Finally, maybe 1e-4 for all devices is not that a bad idea... There is however one thing that bothers me: CI time of mps-tests and cpu-tests (macos-latest).
Now they run during:
We should do something with that:
|
Co-authored-by: vfdev <vfdev.5@gmail.com>
@lrlunin thanks for working on the PR! |
Unfortunately, reduced too much some value:
Can you check this please ? |
Hey @lrlunin, thanks for working on this PR! Is it possible to check the issue: #3345 (comment) and fix it. |
Sorry, I was busy and ill. I increased the minimal dimensions. |
No worries, hope you fill better now. Thanks for the updates! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @lrlunin !
Fixes #3344
Description:
Implements the SSIM for the 3-dimensional structures.
I was forced to reduce the test data dimensions in a way that the
test_ssim_3d
does not consume more memory than the regular 2Dtest_ssim
version.Check list: