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

Cast inputs from uint8 to float before computing metrics #3

Merged
merged 2 commits into from
Dec 30, 2024

Conversation

ulisrael
Copy link
Contributor

@ulisrael ulisrael commented Dec 28, 2024

This PR addresses a bug in the evaluation script for TokenBench. Because videos are read in as uint8 arrays, if the ground truth and reconstructions are subtracted from each other prior to casting to float, the result will potentially under or overflow, changing the resulting value.
For a demonstration of this effect, consider the following simple example:

>>> a = np.array(10, dtype=np.uint8)
>>> b = np.array(8, dtype=np.uint8)
>>> b - a
np.uint8(254)

This manifests in TokenBench's PSNR calculation here:

_FLOAT32_EPS = np.finfo(np.float32).eps
_UINT8_MAX_F = float(np.iinfo(np.uint8).max)
def psnr(pred: _VideoArray, target: _VideoArray):
    mse = ((pred - target) ** 2).mean()  # bug: pred and target are of type np.uint8 here!
    psnr = 20 * np.log10(_UINT8_MAX_F / (np.sqrt(mse) + _FLOAT32_EPS))
    return psnr.item()

For example, for the DV 4x8x8 model, the original vs. fixed PSNR results for the DAVIS dataset are as follows.

Original Fixed
32.98 29.01

To fix the bug, we cast the input videos to float immediately upon loading. The SSIM and rFVD results do not seem affected by this bug.

We really appreciate the work on TokenBench and believe standardizing the evaluation of these video models is incredibly valuable for the community! We kindly request that the reported values in the benchmark be updated to reflect the fixed results. Thank you for all your hard work!

@ulisrael ulisrael changed the title make sure data is cast to float before calculating metrics Cast inputs from uint8 to float before computing metrics Dec 28, 2024
@fitsumreda
Copy link
Collaborator

fitsumreda commented Dec 28, 2024

Thank you for catching this bug! Could you sign your work according to our Contributing guide:
image
I can then merge.

Signed-off-by: ulisrael <[email protected]>
@ulisrael
Copy link
Contributor Author

Amended commit with signoff.

@fitsumreda fitsumreda merged commit c160e96 into NVlabs:main Dec 30, 2024
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