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

fix(sdk): Allow uint8 images to be logged as wandb.Image() #6043

Merged
merged 16 commits into from
Aug 25, 2023

Conversation

nate-wandb
Copy link
Contributor

@nate-wandb nate-wandb commented Aug 10, 2023

Fixes

Description

Images loaded with torchvision.io.read_image would previously crash when logged as a wandb.Image() do to being normalized. This fix converts torch.uint8 images to float prior to logging them to solve this.

Testing

Added test_log_uint8_image() to test_data_types.py. Also passed all other tests in test_data_types.py as well and should only impact images of type torch.uint8. We also logged the randomly created image to wandb.ai with our branch and confirmed that the image created matches what is expected.

🤖 Generated by Copilot at 24f9ac6

Oh we're the wandb crew and we log what we do
With tensors and images of every hue
But when we find a bug in the wandb.Image class
We heave away and fix it with a torch and a cast

@github-actions github-actions bot added cc-fix and removed cc-fix labels Aug 10, 2023
@nate-wandb nate-wandb marked this pull request as ready for review August 11, 2023 15:54
@codecov
Copy link

codecov bot commented Aug 14, 2023

Codecov Report

Merging #6043 (8831416) into main (c82a4ed) will decrease coverage by 0.09%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6043      +/-   ##
==========================================
- Coverage   78.08%   78.00%   -0.09%     
==========================================
  Files         379      379              
  Lines       43518    43520       +2     
==========================================
- Hits        33981    33947      -34     
- Misses       9485     9521      +36     
  Partials       52       52              
Flag Coverage Δ
unittest 81.63% <100.00%> (-0.09%) ⬇️

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

Files Changed Coverage Δ
wandb/sdk/data_types/image.py 87.20% <100.00%> (+1.27%) ⬆️

... and 15 files with indirect coverage changes

@dmitryduev dmitryduev added this to the sdk-2023-09.1 milestone Aug 15, 2023
Copy link
Member

@dmitryduev dmitryduev left a comment

Choose a reason for hiding this comment

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

🎉 nice, @nate-wandb @MBakirWB!! please address the minor comments, and this can go in.
also, feel free to add your future PRs to a future (target) release.

tests/pytest_tests/unit_tests/test_data_types.py Outdated Show resolved Hide resolved
wandb/sdk/data_types/image.py Outdated Show resolved Hide resolved
@dmitryduev
Copy link
Member

Looks like gh is having a hard time triggering circleci, will have to try again once it's working. still, need to fix the linting: https://app.circleci.com/pipelines/github/wandb/wandb/25263/workflows/e058f638-99b4-4d57-af64-80b5c75d3631/jobs/704770

@dmitryduev dmitryduev self-requested a review August 15, 2023 18:33
Copy link
Member

@dmitryduev dmitryduev left a comment

Choose a reason for hiding this comment

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

lgtm. i fixed up the test to use a temp file and a proper way to compare two np arrays

@dmitryduev dmitryduev merged commit 7c74ad5 into main Aug 25, 2023
74 checks passed
@dmitryduev dmitryduev deleted the nate/arty-8bit-torchvision branch August 25, 2023 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants