QUVA-Lab / escnn

Equivariant Steerable CNNs Library for Pytorch https://quva-lab.github.io/escnn/
Other
332 stars 44 forks source link

testimage.jpeg missing #34

Open psteinb opened 1 year ago

psteinb commented 1 year ago

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 https://github.com/QUVA-Lab/escnn/blob/0972a7e458c0de5ea78870f622d374b4c86588e0/escnn/nn/modules/conv/r2convolution.py#L245)

psteinb commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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