Added NCARS and modified loris dependency

[fabrizio-ottati]

Hi! I have added the NCARS dataset and added the try/except test on the import loris line. I also noticed that the sensor coordinates were unsigned in NCARS; hence, as @biphasic suggested to do for CIFAR10DVS, I converted them to unsigned integers. I noticed that another dataset has been removed due to the loris dependency but I decided to not add it, since I and @biphasic talked only about NCARS. Do you think that we should reintegrate it?

[fabrizio-ottati]

Uhm, the checks are failing because loris is not among the requirements. In fact the try/except block works correctly. How can we solve this issue?

[fabrizio-ottati]

Thanks for reviving NCARS! It's good that you brought back the test cases as well. Could you please make sure that the tests are passing on the CI? Right now they fail because loris is not installed. I suggest to add a line 'loris' to test/requirements.txt so that it will be installed before running any tests in the CI pipeline. Thank you!

When I reinstall tonic from conda, it gives me loris as a dependency. It is really strange. I had to force the removal of loris without touching the dependencies. However, even after having added loris to the test dependencies, pytest still fails on the try/except block, triggering the exception.

[biphasic]

so the conda build is somewhat independent of this repo. @Tobias-Fischer is maintaining it and you can see that loris is still in there as a fixed dependency https://github.com/conda-forge/tonic-feedstock/blob/main/recipe/meta.yaml#L24

Even so, the CI (continuous integration) tests are still failing. When you run them on your machine with 'pytest tests' do they work?

[fabrizio-ottati]

When running the test on my machine, having force the uninstallation of loris, I get the same error: the test suite does not install loris event if it is in test/requirements.txt. Here it is a screenshot: Moreover, why is the test on python3.7-ubuntu successfull?

[biphasic]

on your machine you can obviously have loris installed. It's meant as an optional dependency, so that Tonic works also when it's not installed, provided you don't use certain functions/datasets. In the automated test pipeline (CI) on github actions, the machine starts with a clean slate (no packages installed), and then we have to define which ones need to be installed for different use cases. The base requirements.txt lists the fixed dependencies that everyone needs to run basic tonic functionality. test/requirements.txt are installed to test all optional dependencies as well, because the tests need those packages. For example docs/requirements.txt has yet another set of requirements to build documentation, which is not needed to run tonic though. What gets installed is defined here https://github.com/neuromorphs/tonic/blob/develop/.github/workflows/ci-pipeline.yml#L24

So if you have Loris installed on your machine, do all the tests pass?

[biphasic]

the question as to why tests on Ubuntu and Python 3.7 pass is indeed a curious one

[fabrizio-ottati]

on your machine you can obviously have loris installed. It's meant as an optional dependency, so that Tonic works also when it's not installed, provided you don't use certain functions/datasets. [...] test/requirements.txt are installed to test all optional dependencies as well, because the tests need those packages.

Yes, I forced out loris to check if pytest was able to install the dependencies stated in test/requirements.txt, but it does not work. I thought that if I removed loris and I ran pytest, I should have got all the tests passed, since pytest would have installed the dependencies, am I right? But this does not happen on my machine and in the CI.

So if you have Loris installed on your machine, do all the tests pass? With loris installed, all the tests pass. Here's a screenshot.

I did some minor changes with black. All the test are passed. I am going to push it and see what happens, but nothing should change in the CI.

[biphasic]

I ran the tests of your PR on my machine and they also pass. I think the problem is that installing loris on different OS/python versions doesn't always work as intended. This was why I kicked out the package in the past. Even though I tend to avoid this option, I think we might be better off repackaging NCARS in our own hdf5 file and uploading it to one of our servers / Mendeley / wherever. What do you think?

[fabrizio-ottati]

I think it is a good idea! Then, we should do this for all the datasets in which io.read_aedat4() is used, since that function depends on loris, right?

[biphasic]

I think no other dataset depends on this function. Do you think you could do the repackaging? I could look up a host to store the data. For the repackaging:

Use hdf5 format, which is a single file that will store all the data. There is a great package for that called h5py https://docs.h5py.org/en/stable/ You'll need to load all the data using loris in numpy array format and store the samples in an hdf5 file using h5py. Once you have that file I'll upload it to a host and we'll put the new link in the dataset. How does that sound? Gregor

Le 22 avril 2022 08:33:54 GMT+01:00, Fabrizio Ottati @.***> a écrit :

I think it is a good idea! Then, we should do this for all the datasets in which io.read_aedat4() is used, since that function depends on loris, right?

-- Reply to this email directly or view it on GitHub: https://github.com/neuromorphs/tonic/pull/193#issuecomment-1106109503 You are receiving this because you were mentioned.

Message ID: @.***> -- Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.

[fabrizio-ottati]

There is CIFAR10 that depends on loris, for instance. The hd5py option seems fine! Will do!

[fabrizio-ottati]

I would like to point out, given the PR on make_structured_array, that in CIFAR10DVS, at this line, an error is returned because loris already provides a structured array. In particular, the error that I get is:

In [3]: ds[0]
[(('ts', 't'), '<u8'), ('x', '<u2'), ('y', '<u2'), (('p', 'is_increase'), '?')]
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Input In [3], in <cell line: 1>()
----> 1 ds[0]

File ~/miniconda3/envs/dvs/lib/python3.9/site-packages/tonic/datasets/cifar10dvs.py:92, in CIFAR10DVS.__getitem__(self, index)
     90 events = read_aedat4(self.data[index])
     91 print(events.dtype)
---> 92 events = np.lib.recfunctions.unstructured_to_structured(events, self.dtype)
     94 target = self.targets[index]
     96 if self.transform is not None:

File <__array_function__ internals>:5, in unstructured_to_structured(*args, **kwargs)

File ~/miniconda3/envs/dvs/lib/python3.9/site-packages/numpy/lib/recfunctions.py:1074, in unstructured_to_structured(arr, dtype, names, align, copy, casting)
   1071     dts, counts, offsets = zip(*fields)
   1073 if n_elem != sum(counts):
-> 1074     raise ValueError('The length of the last dimension of arr must '
   1075                      'be equal to the number of fields in dtype')
   1076 out_dtype = dtype
   1077 if align and not out_dtype.isalignedstruct:

ValueError: The length of the last dimension of arr must be equal to the number of fields in dtype

Hence, loris hits again. For this reason I told you @biphasic that we should repackage CIFAR10 in addition to NCARS.

[fabrizio-ottati]

Hi @biphasic. I have packaged both NCARS and CIFAR10DVS in HDF5 format, as you suggested, removing the loris dependency. Where can I upload these files?

[biphasic]

hey @fabhertz95 thanks a lot, I haven't forgotten about this. I have a deadline on Thursday so a bit busy at the moment, but I'll look up a potential host on Friday :)

[fabrizio-ottati]

Don't worry @biphasic. I googled a little bit and I found out that it is possible to upload the dataset on Zenodo, which is pretty reliable. I was wondering about how to handle the copyrighting, since those are not our datasets.

I also found out some months ago about N-ImageNet. Would it be a good idea to include it into Tonic? I will probably never use it (too large for my computational capabilities :) ) but someone might want to.

[biphasic]

thank you @fabhertz95. You make a very good point. Uploading the data ourselves could get us in trouble. Rethinking our options, I think I'm going to write to Prophesee to ask them if they would repackage some of their data and work with us to provide their datasets more easily than in their custom format. I will let you know how that goes. Regarding N-ImageNet, I'm not too keen on adding another dataset that is converted from images. I'm not against adding it, just saying that I don't consider it a high priority.

[fabrizio-ottati]

@biphasic I totally agree with you on both points. Let's see how it goes with them. Meanwhile, CIFAR10 and NCARS can be used with loris as an external dependency, so there is no rush. If someone needs it, I'll provide the script with the Tonic and loris code that I used.

[gicu8ab2]

FWIW, for those who want to use CIFAR10-DVS without loris the following worked for me where I just wrote my own read_aedat4() function and used aedat module:

def read_aedat4(in_file): import aedat decoder = aedat.Decoder(in_file) packet = next(iter(decoder)) events = packet["events"] return events