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

Enhancement: Added interface for GMSD and Multi-Scale GMSD #151

Merged
merged 6 commits into from
Jul 24, 2020

Conversation

denproc
Copy link
Collaborator

@denproc denproc commented Jul 23, 2020

Proposed Changes

  • Added interface for GMSD and Multi-Scale GMSD to use them as functions without creating the Layer (also to match other metrics, which are available as function and as nn.Layer);
  • Optimised colour conversion;
  • Optimised tests, checked on cuda.

@denproc denproc requested review from zakajd and snk4tr July 23, 2020 17:03
@codecov
Copy link

codecov bot commented Jul 23, 2020

Codecov Report

Merging #151 into master will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #151      +/-   ##
==========================================
+ Coverage   94.54%   94.64%   +0.09%     
==========================================
  Files          24       24              
  Lines        1613     1624      +11     
==========================================
+ Hits         1525     1537      +12     
+ Misses         88       87       -1     
Flag Coverage Δ
#unittests 94.64% <100.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
piq/gmsd.py 100.00% <100.00%> (+1.12%) ⬆️

Copy link
Collaborator

@zakajd zakajd left a comment

Choose a reason for hiding this comment

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

Nice! Liked that you added a lot of test.
Maybe we can also add test to check consistency with MATLAB implementation? Code is available here

piq/gmsd.py Outdated Show resolved Hide resolved
piq/gmsd.py Outdated Show resolved Hide resolved
piq/gmsd.py Outdated Show resolved Hide resolved
piq/gmsd.py Outdated Show resolved Hide resolved
- tests to validate values with MATLAB results
- changed the valid colour transfer of the images before any pooling
- changes proposed by @zalajd
zakajd
zakajd previously approved these changes Jul 24, 2020
Copy link
Collaborator

@zakajd zakajd left a comment

Choose a reason for hiding this comment

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

Now it's even better 🚀
Approve

Copy link
Contributor

@snk4tr snk4tr left a comment

Choose a reason for hiding this comment

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

Don't look at the number of comments, they are very minor ones 😉

piq/gmsd.py Outdated Show resolved Hide resolved
piq/gmsd.py Outdated Show resolved Hide resolved
tests/test_gmsd.py Outdated Show resolved Hide resolved
tests/test_gmsd.py Outdated Show resolved Hide resolved
tests/test_gmsd.py Outdated Show resolved Hide resolved
tests/test_gmsd.py Outdated Show resolved Hide resolved
tests/test_gmsd.py Outdated Show resolved Hide resolved
tests/test_gmsd.py Outdated Show resolved Hide resolved
tests/test_gmsd.py Outdated Show resolved Hide resolved
tests/test_gmsd.py Outdated Show resolved Hide resolved
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@denproc denproc requested a review from snk4tr July 24, 2020 18:12
@denproc
Copy link
Collaborator Author

denproc commented Jul 24, 2020

@snk4tr ready to merge

@snk4tr snk4tr merged commit 5ec2757 into master Jul 24, 2020
@denproc denproc deleted the enhancement/gmsd branch July 25, 2020 14:04
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.

3 participants