Repository navigation
fix(activations): run each stimulus once without require_variance - #2628
Open
jig-mistry wants to merge 1 commit into
Open
jig-mistry wants to merge 1 commit into
jig-mistry wants to merge 1 commit into
Conversation
`_get_activations_batched` always ran each stimulus `_microsaccade_helper.number_of_trials` times, but that count is only updated by `require_variance=True` calls. After a call with `require_variance=True, number_of_trials=3`, the next call without `require_variance` on the same model still ran 3 passes per stimulus, while `_package_layer` expected 1, and failed with an AssertionError. Check `require_variance` there, as `_package_layer` and `attach_stimulus_set_meta` already do. This covers `__call__` as well as direct `from_paths` / `from_stimulus_set` calls. Closes brain-score#1167
This was referenced Nov 2, 2026
KartikP
pushed a commit
that referenced
this pull request
Nov 5, 2026
Closes #376. With an empty list of paths or an empty `StimulusSet`, nothing complains until packaging. `_get_activations_batched` never enters its batch loop, so `layer_activations` stays `None`, and `_package` fails with an error that says nothing about the input: ``` File ".../brainscore_vision/model_helpers/activations/core.py", line 369, in _package shapes = [a.shape for a in layer_activations.values()] AttributeError: 'NoneType' object has no attribute 'values' ``` With stored activations it can be worse. Their key ignores the paths, so an empty request under a stimuli identifier that is already stored gets the stored activations back. With a list of paths that is a full assembly for zero stimuli. With a `StimulusSet` it fails later, in `attach_stimulus_set_meta`. This adds a check at the start of `from_paths`, before the stored activations are looked up: ```python if len(stimuli_paths) == 0: raise ValueError("No stimuli passed to retrieve activations from") ``` `__call__` and `from_stimulus_set` both go through `from_paths`, so all entry points are covered, with or without `require_variance`. The message matches the existing "No layers passed to retrieve activations from" check in `_from_paths`. Test: `test_no_stimuli` passes an empty list and an empty `StimulusSet` (filtered down to zero rows) through `__call__`, with `require_variance` off and on. It also replaces `_from_paths_stored` and `_from_paths` with stubs that fail if called, so the check is shown to come before both computing and loading activations. It fails on `master` and passes here. This doesn't overlap with #2628 or #2629. They touch other parts of the same file and merge cleanly with this in either order.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1167.
After a model has been called with
require_variance=Trueand more than one trial, the next call withoutrequire_variancefails:__call__only updates_microsaccade_helper.number_of_trialsonrequire_variance=Truecalls, so the count from the last such call sticks around._get_activations_batchedthen runs every stimulus that many times, even withrequire_varianceoff. But_package_layerexpects one run per stimulus.Two ways to get there:
require_variance=Trueand then callslook_atwithout it;Existing scores aren't affected. The bug raises rather than returning wrong activations, and scoring rarely reaches it:
require_variance, and it uses it for both fitting and testing.brainscore_vision.scorecallsload_modelfor each benchmark, and nearly every plugin builds a new extractor there. (resnet50-moclr8degis an exception: it builds its wrapper at import and reuses it.)The fix:
_get_activations_batchednow only repeats stimuli whenrequire_varianceis set, as_package_layerandattach_stimulus_set_metaalready do. This covers__call__and directfrom_paths/from_stimulus_setcalls alike. Calls withrequire_variance=Truethat work onmastergive the same results.I went with this rather than resetting the count in
__call__, as the issue suggests, for two reasons:from_paths/from_stimulus_setcalls.LayerPCAfits itself by calling the model again from inside a batch hook (pca.py:47). If that happens during arequire_variance=Truecall, the nested call fails onmastertoo, because it inherits the outer trial count. This change fixes that case. A reset would only move the failure to the outer call, since the nested call would reset the count under it.Test:
test_number_of_trials_does_not_carry_overmakes arequire_variance=Truecall, then calls the same model without it through__call__,from_pathsandfrom_stimulus_set. It fails onmasterwith the error above and passes here.Related: the stored-activations cache key always records
number_of_trials=1, whatever the actual trial count. That's in a separate PR, #2629, since it changes the cache keys ofrequire_variance=Truerequests.