NeuroBench / neurobench

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

Nehar example #96

Closed ben9809 closed 9 months ago

gurgese commented 11 months ago

@jasonlyik The NeHAR example is ready could you review this PR?

jasonlyik commented 9 months ago

Hey @ben9809, apologies that it took a while but I got around to reviewing the NeHAR example. All looks good except that I can't run the SCNN from my M1 Mac since there is some pytorch issue.

Remaining are very small changes for the ipynb which I've commented.

ben9809 commented 9 months ago

Hello @jasonlyik, are you encountering issues specifically related to the training script on your Mac?

jasonlyik commented 9 months ago

@ben9809 The error appears to be a conv issue on M1 Mac, anytime the forward pass is called. I can reproduce the same error with the simple script:

conv = nn.Conv1d(6, 200, 2, padding=4)
x = torch.rand(256, 6, 1)
y = conv(x)

So training or inference of the CSNN architecture is not possible. I could train and run the other two models in the notebook though.

ben9809 commented 9 months ago

Thanks for the details. It appears that the issue is specific to the M1 architecture. I just tried with an M3 Mac and it works.

ben9809 commented 9 months ago

Hey @jasonlyik, Unfortunately, I can't see the comments on the IPython notebook you referred to. Could you please direct me to the section you're referring to? This will help me address the changes accurately.

jasonlyik commented 9 months ago

@ben9809 I didn't associate them with any line numbers, copy pasting them here:

ben9809 commented 9 months ago

Thank you @jasonlyik!

In order to maintain compatibility with the version from PyPI (used in Google Colab) and the version in the dev branch, I've decided to omit "synaptic_operations" as a metric extracted from the different models in the notebook for now.

Let me know what you think about this decision.

jasonlyik commented 9 months ago

Sounds good, we have some other changes planned though for the v1.0 release so I may go through and touch up later as well.

I'll verify and pull this in, thanks!