-
Notifications
You must be signed in to change notification settings - Fork 80
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
Add microsaccades to Brain-Score core #492
Add microsaccades to Brain-Score core #492
Conversation
It seems that the Jenkins build is skipping tests on this PR, which is not the intended case:
Unsure as to why this happens, and whether it's something I can fix as a part of the PR? The intended case certainly is for tests to run on this PR. Any help would be much appreciated! |
Hi @benlonnqvist - thanks for flagging this; it looks like you've caught an issue with the way we're handling forked PRs in Jenkins. This should raise an assertion error in the Travis PR build, but that isn't able to run while there are unresolved conflicts with |
Thank you @kvfairchild! Much appreciated. |
Hi @benlonnqvist - can you try again now? |
Hi @kvfairchild, tests seem to be working now, thanks again! |
Hi! I think there are some remaining tests that fail on Jenkins that I don't think have anything to do with the PR itself - although I could be wrong. Perhaps it might be possible to have a quick look as to what might be going wrong with the few remaining tests and whether they have anything to do with this PR specifically? Thanks! |
Hi, it seems that Jenkins is now crashing on building the PR altogether (see here). I don't think this is related to the PR - is there something I can do to resolve the issue potentially? |
re-trigger jenkins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in addition to in-code comments, please delete brainscore_vision/data/scialom2024/__init__.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good once comments are addressed and tests pass. Especially adding unit tests for the exact values are essential imo.
Co-authored-by: Martin Schrimpf <[email protected]>
… tests run for, while keeping the test the same
All tests on Jenkins (and Travis) are green. Travis reports a failure because it gets killed (likely runtime, unrelated to this PR) |
* email fix for score_plugins workflow (#581) * Emails passed to jenkins * Update .github/workflows/score_new_plugins.yml Co-authored-by: Katherine Fairchild <[email protected]> * fix deprecated set output --------- Co-authored-by: Katherine Fairchild <[email protected]> * add eBarlow_Vanilla to models (#587) Co-authored-by: AutoJenkins <[email protected]> * add eBarlow_lmda_01 to models (#588) Co-authored-by: AutoJenkins <[email protected]> * add cornet_s_0_0_0 to models (#589) Co-authored-by: AutoJenkins <[email protected]> * add unet_ethan to models (#590) Co-authored-by: AutoJenkins <[email protected]> * add unet_ethan to models (#591) Co-authored-by: AutoJenkins <[email protected]> * Revert "add unet_ethan to models (#591)" (#594) This reverts commit c89059e. * Revert "add unet_ethan to models (#590)" (#595) This reverts commit 9a8774a. * Transfer plugin information by artifact for scoring runs (#592) * upload and download json artifact between jobs * fix testing changes * brain-score.org (user:514) | Add new plugin(s): models: [cornet_s_0_0_0 cornet_s_0_1_0] | (public:False) (#603) * add cornet_s_0_0_0 to models * add cornet_s_0_1_0 to models --------- Co-authored-by: AutoJenkins <[email protected]> * Add microsaccades: interface and default model helper (#492) * integrate brain-score/model-tools#75 to brain-score/vision * add exception for when temp file write fails * fix error with tf temp file management * add bandaid to a DataAssembly index problem * move microsaccades to the model side * update comments * remove indexing bug * fix bug with activations.shape * Apply suggestions from code review Co-authored-by: Martin Schrimpf <[email protected]> * Delete brainscore_vision/data/scialom2024/__init__.py * address review changes * remove needless import * fix bug with temporary file handling test * assume number_of_trials=1 and require_variance=False when getting stored activations * fix bug with access to ActivationsExtractorHelper.set_visual_degrees * move extractor calls to ModelCommitment generic * add check for whether activations_model exists * fix bug with TestVisualDegrees * add link to BrainModel issue * remove shifts from stimulus set packaging * change link signatures * change microsaccade call signature * microsaccades are now computed on both a pixel and degree basis * Apply suggestions from code review Co-authored-by: Martin Schrimpf <[email protected]> * fix outdated comments, type hints, etc. * change function call to reduce repetition * add kwargs to microsaccade helpers * refactor microsaccade usage to their own class to improve readability * refactor microsaccade coords into MicrosaccadeHelper * refactor microsaccade building * change the way MultiIndex is set * fix tf/pytorch/keras bug with image shape calculation * cv2 reshaping in translate * add test for exact microsaccades * fix microsaccade indexing * rename test to be in line with current naming * add require_variance to _from_paths_stored * reduce unnecessarily long test times by reducing the number of trials tests run for, while keeping the test the same --------- Co-authored-by: Martin Schrimpf <[email protected]> * Use MajajHong2015public.IT instead of mock data (#585) * Changed _MockBenchmark to new format * revert ceiler back to martin's suggestion * implemented MockBenchmark to use MJ2015 Public --------- Co-authored-by: Katherine Fairchild <[email protected]> Co-authored-by: AutoJenkins <[email protected]> Co-authored-by: Ben Lonnqvist <[email protected]> Co-authored-by: Martin Schrimpf <[email protected]> Co-authored-by: Michael Ferguson <[email protected]>
* add callable microsaccades * add test to catch errors in stimulus set meta attachment when using microsaccades * mark new test as memory intense * Apply suggestions from code review Co-authored-by: Martin Schrimpf <[email protected]> * add require_variance to the signature of look_at for all behavioral arbiters (required since the generic look_at no longer takes kwargs but specifically require_variance) --------- Co-authored-by: Martin Schrimpf <[email protected]>
In this PR we add microsaccades to Brain-Score core. Integration of model_tools PR #75 to Brain-Score 2.0.
Conceptual Problem: We need variance in model responses to static stimuli.
Conceptual Solution: Microsaccades are a prevalent phenomenon in vision and visual experiments. They are involuntary movements of the eye at very small scales. Microsaccades are not (and often can not due to their small extent) be controlled for in experiments, and are thus implicitly baked into the variance in primate responses. As such, microsaccades are a phenomenon that is either explicitly or implicitly believed to be unimportant for behavioral/neural outcomes, and thus a good candidate for getting variance in responses to static stimuli. (e.g. Rolfs 2009, Vis. Res.; Haddad & Steinmann 1973, Vis. Res.)
Technical problem: The current Brain-Score interface does not support microsaccades.
Technical solution: the ActivationsExtractorHelper will be changed. The from_paths method now takes a path, loads the image from the path, does pixel position jittering to the image, and saves the jittered image into a temporary new path. The new path is passed to the downstream functions to get the model activations.
To accomplish this, the from_paths method will now receive a dictionary of tags. This dictionary of tags comes from a call to the look_at method, where number_of_trials is replaced with a new dictionary. This new dictionary then traverses the following path:
TODO list:
Other Brain-Score benchmark PRs depend on this PR (#365).
Huge thanks to Hannes Mehrer (@hanme) who implemented microsaccades, which I've only adapted for the generalized case here.