-
Notifications
You must be signed in to change notification settings - Fork 956
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
Migrate to nv-flip for image comparison #3830
Conversation
7488aba
to
c81e527
Compare
c81e527
to
a6c6ad7
Compare
07dffad
to
b3cee92
Compare
b3cee92
to
f4e997b
Compare
f41c8b9
to
1ef77e0
Compare
1ef77e0
to
e7df29e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. What color space is nvflip expecting the data to be in? It should be documented in the nv-flip crate and we should make sure the tests actually produce pixels in the right color space otherwise we are only giving the illusion of having a principled error metric.
Filed the color space info here: gfx-rs/nv-flip-rs#1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@cwfitzgerald there seems to be an unexpected test pass 👀 |
Yeah, that was #3809 landing - will remove the failures. |
Checklist
cargo clippy
.RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown
if applicable.Connections
Resolves #2761
Description
Integrates NVlab's ꟻLIP for image comparison, using their various weighted histogram methods. This feels soooo much more stable, thresholds aren't insanity, and new devices seem to consistently fall under threshold.
I also fixed up some other issues with the tests.
Testing
I ran the tests on all of my machines' cards and they all pass.