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

testimage.jpeg missing #34

Open
psteinb opened this issue Feb 23, 2023 · 5 comments
Open

testimage.jpeg missing #34

psteinb opened this issue Feb 23, 2023 · 5 comments

Comments

@psteinb
Copy link
Contributor

psteinb commented Feb 23, 2023

Just wanted to run the test suite to double check #29, but then I ran into another problem:

        if filename:
>           fp = builtins.open(filename, "rb")
E           FileNotFoundError: [Errno 2] No such file or directory: '../group/testimage.jpeg'

which ocurred in escnn/nn/modules/conv/r2convolution.py:245 (see

x = mpimg.imread('../group/testimage.jpeg').transpose((2, 0, 1))[np.newaxis, 0:c, :, :]
)

@psteinb
Copy link
Contributor Author

psteinb commented Feb 23, 2023

On this note, I must stress that having code (and data and dependencies) that is mostly used in the test suite in production code, can cause a lot of maintenance harm very quickly. I would advise to make the use of testimage.jpeg optional or come up with a testimage that can quickly be generated through some lines of numpy. At best, the testimage code is moved into the test suite and declared as default or something along these lines.

@Gabri95
Copy link
Collaborator

Gabri95 commented Feb 23, 2023

Hi @psteinb

Thanks for pointing this out! Do you have any recommendations on how to deal with this?

I did not want to use a randomly generated image with numpy 1) to ensure consistency over time and 2) to check equivariance on data representing natural images rather than random noise. I also thought it was not a good idea to include images inside the library.

Maybe I will try to use scipy.misc.face() as a testing image

Thanks,
Gabriele

@psteinb
Copy link
Contributor Author

psteinb commented Feb 23, 2023

It is hard for me to comment, as I don't know the content of testimage.jpeg and what it's purpose is. For such test data, indeed scipy.misc.face or any of the skimage.data entries will do the trick. For the time being, this is the safest option, I think.

But I still feel troubled that this code resides in the main escnn folder. It would be great to refactor the check_equivariance function to an interface like:

def check_equivariance(self, testimage: np.ndarray, atol: float = 0.1, rtol: float = 0.1, assertion: bool = True, verbose: bool = True):

Then in the test folder, you could compose a python module that provides the testimage for example. This would disentangle this more.

@Gabri95
Copy link
Collaborator

Gabri95 commented Feb 24, 2023

I see your point!
I have replaced the image with scipy.datasets.face for the moment, but will try to move all the equivariance-check routines in the test folder in a later release (I don't have time to perform this large refactoring in this moment, but will come back to that as soon as possible).

Thanks again for the useful advices and feedback!

Gabriele

@chAwater
Copy link
Contributor

BTW, this file can be found in the e2cnn repo (link).

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

3 participants