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

change dependency to pyrtools? #2

Open
maitra opened this issue Oct 8, 2023 · 7 comments
Open

change dependency to pyrtools? #2

maitra opened this issue Oct 8, 2023 · 7 comments

Comments

@maitra
Copy link

maitra commented Oct 8, 2023

According to the link https://github.com/LabForComputationalVision/pyPyrTools pyPyrTools is unsupported and replaced by pyrtools? Should the dependency and code be modified to reflect that?

@maitra
Copy link
Author

maitra commented Oct 8, 2023

I wonder what the answer of the test.py is? I get: 0.19145732 as the answer in the test example. Is that the correct answer?

@pavancm
Copy link
Owner

pavancm commented Oct 9, 2023

I wonder what the answer of the test.py is? I get: 0.19145732 as the answer in the test example. Is that the correct answer?

The gold standard way to check the correctness is to use the code provided by the authors available here. As long as the code does not throw any error the answer you're getting should be correct I believe.

According to the link https://github.com/LabForComputationalVision/pyPyrTools pyPyrTools is unsupported and replaced by pyrtools? Should the dependency and code be modified to reflect that?

I need to look into this. If you have a working code can you submit a pull request? I will verify and merge into the repo. Thanks!

@maitra
Copy link
Author

maitra commented Oct 9, 2023

I wonder what the answer of the test.py is? I get: 0.19145732 as the answer in the test example. Is that the correct answer?

The gold standard way to check the correctness is to use the code provided by the authors available here. As long as the code does not throw any error the answer you're getting should be correct I believe.

I do not have Matlab so can not run the "gold standard code." However, I note that if I convert the bmp images to png, I get a different answer. Is this correct? Do all files only need to be .bmp?

According to the link https://github.com/LabForComputationalVision/pyPyrTools pyPyrTools is unsupported and replaced by pyrtools? Should the dependency and code be modified to reflect that?

I need to look into this. If you have a working code can you submit a pull request? I will verify and merge into the repo. Thanks!

I have redone your code to be in python3, however, it is now slightly different and no longer depends on any of these two libraries (since I only modified those parts that are called by the code, and dropped the rest). So, not sure how useful it will be to you.

@pavancm
Copy link
Owner

pavancm commented Oct 9, 2023

However, I note that if I convert the bmp images to png, I get a different answer. Is this correct? Do all files only need to be .bmp?

How big is the difference? Technically it should not change significantly unless the conversion is causing significant changes in the pixel values. Try checking whether the pixel values present in .bmp and .png formats are very different.

@maitra
Copy link
Author

maitra commented Oct 9, 2023

However, I note that if I convert the bmp images to png, I get a different answer. Is this correct? Do all files only need to be .bmp?

How big is the difference? Technically it should not change significantly unless the conversion is causing significant changes in the pixel values. Try checking whether the pixel values present in .bmp and .png formats are very different.

Massive.

0.1914 vs. 0.07.

The bmp and png figures look the same. conveting back the png to bmp brings it back to 0.1914. I do not understand this. Do you also get 0.1914 on the example that you have above?

Assuming that there are no errors in the code, that is why I was wondering if your answer matched the one that you would get using the Matlab code.

It would be helpful to have an example with the answer, otherwise the implementation is a bit uncertain as to whether it compiled correctly..

@maitra
Copy link
Author

maitra commented Oct 9, 2023

i cleaned up the C code (convolve.c) a bit to bring it on par with ansi pedantic (and get no warnings, boy this is 1987 code!) and get the same answer now, but very low. I get 0.07073818 as VIF. Is that what the python2 and matlab codes also give you?

The above is with M=3 in vifvec.py. Increasing M to 32 (which is the highest before I start getting a singular matrix) gives me a value of 0.08415761.

I hope the code is doing the right thing.

I have now forked and put the Python3 code up in case you wanted to check it out.

@pavancm
Copy link
Owner

pavancm commented Oct 9, 2023

I am currently out of town. I will verify these observations once I am back. Thanks for pointing this!

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

No branches or pull requests

2 participants