NeuroBench / neurobench

Benchmark harness and baseline results for the NeuroBench algorithm track.
https://neurobench.readthedocs.io
Apache License 2.0
47 stars 12 forks source link

New prototypical definition for MSWC FSCIL task #150

Closed jasonlyik closed 6 months ago

jasonlyik commented 7 months ago

V1 of the MSWC FSCIL task demo/example code.

Also neurobench/preprocessing/speech2spikes.py was edited (the only file not from the neurobench/examples directory) to include passing the self.threshold to tensor_to_events to fix a "bug" because else the threshold parameter from S2S is never used.

jasonlyik commented 7 months ago

@Maxtimer97

Hey Maxime,

Noah, Korneel, and I are working on the v1.0 open release of the harness. I noticed that there doesn't seem to be any pretrained models with use_readout_bias = True uploaded currently, could you upload one CNN and one SNN model that replaces all of the others?

I am putting together the checklist of things to cleanup below, but I can cover all of the points quickly if you upload the two pretrained models.

jasonlyik commented 7 months ago

Checklist for this branch --> dev:

Final check: Make sure this PR changes nothing outside of the neurobench/examples folder, so that nothing will change from the pip package

V0XNIHILI commented 7 months ago

@Maxtimer97 I slightly restructured your code here: #158

Maxtimer97 commented 7 months ago

Hey Jason and Douwe,

I just pushed the last changes on fscil_snn with the current reference models for the prototype method added to model_data in this commit : https://github.com/NeuroBench/neurobench/commit/81845042386d5222f18aec1988817b3ba3379c72 (mswc_cnn_proto and mswc_rsnn_proto).

Then in general the code is indeed still not clean. I created an extra branch for myself to keep running experiments to try to push the SNN performance a bit further. So we can stay on fscil_snn to clean everything and add it to the v1.0 .

But else I think what you noted should be good, the soft_delta was a test but indeed didn't work well. Probably there are also some unused variables from the weight consolidation time.

But so is it OK for you to do it @jasonlyik ? Maybe if you clean it quickly I can make sure it runs well again and do a final check :)

V0XNIHILI commented 7 months ago

@Maxtimer97 I can pick this up. A few questions:

Maxtimer97 commented 7 months ago

@Maxtimer97 I can pick this up. A few questions:

  • Which models under model_data do you want to keep?

mswc_cnn_proto and mswc_rsnn_proto are the ones for now

  • Should we keep the visualization.ipynb notebook?

No it is a debugging tool

  • For the softdelta option, do I just remove passing this argument to S2S? And that's it?

Yes, but also remove these lines here: https://github.com/NeuroBench/neurobench/blob/06fc71ecad6f0896f3ca301bbf6bf411e350e3b9/neurobench/preprocessing/speech2spikes.py#L64C1-L69C66

  • What should I write in the tutorial about downloading the data? Should I provide the Google Drive link in the tutorial?

Not sure, what is the plan for hosting the dataset now @jasonlyik ? Does the google drive have the csv files already?

V0XNIHILI commented 7 months ago

To do:

V0XNIHILI commented 7 months ago

@Maxtimer97 is the value 10 here the number of ways per session? https://github.com/NeuroBench/neurobench/blob/cca2e7216434d6fa0d14e837065343333deadf87/neurobench/examples/mswc_fscil/mswc_fscil_proto.py#L385

If so, should we move all hardcoded values of that 10 by a variable for improved clarity?

V0XNIHILI commented 7 months ago

@Maxtimer97 is there a specific reason for re-instantiating the base testing set every session:

https://github.com/NeuroBench/neurobench/blob/cca2e7216434d6fa0d14e837065343333deadf87/neurobench/examples/mswc_fscil/mswc_fscil_proto.py#L308

Or can I just move this statement outside of the session for-loop?

V0XNIHILI commented 7 months ago

@Maxtimer97 could you have a look at the new tutorial and tell me what you think?

Maxtimer97 commented 7 months ago

@Maxtimer97 is the value 10 here the number of ways per session?

https://github.com/NeuroBench/neurobench/blob/cca2e7216434d6fa0d14e837065343333deadf87/neurobench/examples/mswc_fscil/mswc_fscil_proto.py#L385

If so, should we move all hardcoded values of that 10 by a variable for improved clarity?

Yes 10 is for the number of ways per session as we need to update the weights of the last 10 query classes (new session). So I guess it should rather be a capital letter parameter like NUM_WAYS for clarity indeed :)

Maxtimer97 commented 7 months ago

@Maxtimer97 is there a specific reason for re-instantiating the base testing set every session:

https://github.com/NeuroBench/neurobench/blob/cca2e7216434d6fa0d14e837065343333deadf87/neurobench/examples/mswc_fscil/mswc_fscil_proto.py#L308

Or can I just move this statement outside of the session for-loop?

You can just move it outside!

Maxtimer97 commented 7 months ago

@Maxtimer97 could you have a look at the new tutorial and tell me what you think?

The tutorial looks better! Maybe we can still split the final block in more blocks to explain what each session contains and what the prototypical approach is doing each time. I can take care of that tomorrow afternoon :)

V0XNIHILI commented 6 months ago

Nice!!