photosynthesis-team / piq

Measures and metrics for image2image tasks. PyTorch.
Apache License 2.0
1.41k stars 120 forks source link

Create variables on correct devices and with correct types #292

Closed zakajd closed 2 years ago

zakajd commented 2 years ago

Related to #288

Proposed Changes

codecov[bot] commented 2 years ago

Codecov Report

Merging #292 (27f9cab) into master (19f0d92) will decrease coverage by 2.33%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #292      +/-   ##
==========================================
- Coverage   93.35%   91.02%   -2.34%     
==========================================
  Files          33       33              
  Lines        2289     2284       -5     
==========================================
- Hits         2137     2079      -58     
- Misses        152      205      +53     
Flag Coverage Δ
unittests 91.02% <100.00%> (-2.34%) :arrow_down:

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

Impacted Files Coverage Δ
piq/brisque.py 99.01% <100.00%> (ø)
piq/dss.py 100.00% <100.00%> (+1.03%) :arrow_up:
piq/fsim.py 100.00% <100.00%> (ø)
piq/functional/filters.py 100.00% <100.00%> (ø)
piq/ms_ssim.py 100.00% <100.00%> (ø)
piq/ssim.py 100.00% <100.00%> (ø)
piq/vsi.py 100.00% <100.00%> (ø)
piq/gs.py 32.96% <0.00%> (-59.35%) :arrow_down:
sonarcloud[bot] commented 2 years ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

zakajd commented 2 years ago

As discussed with Denis and Sergei, we won't create kernels as torch.DoubleTensor by default.

Reasoning: It's much more common to have single precision float tensors as inputs. So having kernels in float64 will create a lot of casting back to float32 every time metrics are computed.

This PR adds support for float64 input inference and slight loss of precision due to kernel's dtype is not considered important.

@denproc please review again and merge.