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

Tutorial for 2d cores added #233

Merged
merged 6 commits into from
Mar 12, 2024

Conversation

neuronmorph
Copy link
Contributor

Tutorial provides examples on how to use different 2d cores for sensorium 2022

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link

codecov bot commented Mar 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 7.51%. Comparing base (1323caa) to head (185cd90).

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #233   +/-   ##
=====================================
  Coverage   7.51%   7.51%           
=====================================
  Files         58      58           
  Lines       5978    5978           
  Branches    1018    1018           
=====================================
  Hits         449     449           
  Misses      5492    5492           
  Partials      37      37           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pollytur
Copy link
Collaborator

pollytur commented Mar 8, 2024

Based on discussion with Konsti

  • small configs for each core type, highlighting the differences
  • core_config not model_config
  • change a bit the description for the SE core
  • should loading and not loading a ppretrain core for transfer learning

Copy link
Contributor

@KonstantinWilleke KonstantinWilleke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice PR! I have another request that would be nice to add:
Could you add to the top of the notebook a small description of Sensorium and the dependencies.

So that you link to the sensorium 2022 repo so that it is clear which dataset is being used. And there is also the dependecy on the sensorium package:
dataset_fn = 'sensorium.datasets.static_loaders'
This line requires that the sensorium package is installed.

A possible solution is: you can describe that if they have sensorium installed, they can use this data. But in case the user wants to proceed without it, you can just initialize a torch.tensor (like torch.randn(32,1,144,256)) which has the same size. And then the user does not need "real" images, but just a tensor that is essentially the same.

@pollytur
Copy link
Collaborator

pollytur commented Mar 9, 2024

@KonstantinWilleke
The installation is there
image

But I think the import (like import sensorium is missing)

I actually would disagree with the explanation of sensorium dependencies, as it's not the goal of this tutorial- the goal is to show the cores. And I expect for some people to be very distracted by this from the beginning.

Using random inputs is actually a great idea and would make things easier and more disentangled. This way we do not need either installation or explanations.

pollytur

This comment was marked as outdated.

Copy link
Collaborator

@pollytur pollytur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please update it with the random input for the cores as Konsti suggested (removing the dataset creation and sensorium installation) and then we could merge this great example?

@neuronmorph
Copy link
Contributor Author

Please have a look. I removed the Sensorium 2022 dependencies.

@pollytur
Copy link
Collaborator

Great, thanks a lot!

@KonstantinWilleke
Copy link
Contributor

Great work @neuronmorph! Thanks a lot. You can merge the PR - all that is left to do is to update the branch. You can click the button "update branch" and then feel free to merge.

@pollytur pollytur merged commit 736ca41 into sinzlab:main Mar 12, 2024
7 checks passed
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

Successfully merging this pull request may close these issues.

3 participants