photosynthesis-team / piq

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

PR strange behaviour #285

Closed merunes-goldman closed 2 years ago

merunes-goldman commented 2 years ago

test code

from typing import Dict

import torch
import torch.utils.data as data_utils

from piq import PR as _PR
from piq.functional.base import crop_patches

class _ComputeFeatsDataset(data_utils.Dataset[Dict[str, torch.Tensor]]):
    def __init__(self, batch: torch.Tensor) -> None:
        self.batch = crop_patches(batch)

    def __getitem__(self, index: int) -> Dict[str, torch.Tensor]:
        return {'images': self.batch[index]}

    def __len__(self) -> int:
        return int(self.batch.shape[0])

def _main() -> None:
    reference = torch.ones(1, 3, 256, 256)
    distorted = reference

    reference_data_loader = data_utils.DataLoader(_ComputeFeatsDataset(reference))
    distorted_data_loader = data_utils.DataLoader(_ComputeFeatsDataset(distorted))

    pr_ = _PR()
    reference_features = pr_.compute_feats(reference_data_loader, device='cpu')
    distorted_features = pr_.compute_feats(distorted_data_loader, device='cpu')
    score = pr_(reference_features, distorted_features)

    precision, recall = float(score[0].numpy()), float(score[1].numpy())

    print(f"{precision=}")
    print(f"{recall=}")

if __name__ == '__main__':
    _main()

prints

precision=0.0
recall=0.9795918464660645

but they seems to be

precision=1.0
recall=1.0

i found potentially mismatching with paper, original formula

image

actual formula

        precision = (
                distance_real_fake < real_nearest_neighbour_distances.unsqueeze(1)
        ).any(dim=0).float().mean()

        recall = (
                distance_real_fake < fake_nearest_neighbour_distances.unsqueeze(0)
        ).any(dim=1).float().mean()

so, < seems to be <=

commit in pull request is checking <= with precision (due to float errors)

PS: (offtop) in paper they define additional metric "Realism Score" - are there plans to add it? Is that even necessary? I may try to add Realism Score as another pull request later.

zakajd commented 2 years ago

in paper they define additional metric "Realism Score" - are there plans to add it? Is that even necessary? I may try to add Realism Score as another pull request later.

We didn't plan to add Realism Score, but if you wish to open a PR it's always welcomed 👌🏼

codecov[bot] commented 2 years ago

Codecov Report

Merging #285 (0a5ac0f) into master (79cccf8) will decrease coverage by 2.27%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #285      +/-   ##
==========================================
- Coverage   93.25%   90.97%   -2.28%     
==========================================
  Files          33       33              
  Lines        2283     2283              
==========================================
- Hits         2129     2077      -52     
- Misses        154      206      +52     
Flag Coverage Δ
unittests 90.97% <100.00%> (-2.28%) :arrow_down:

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

Impacted Files Coverage Δ
piq/pr.py 100.00% <100.00%> (ø)
piq/gs.py 32.96% <0.00%> (-57.15%) :arrow_down:
snk4tr commented 2 years ago

I will have a look ASAP.

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