brain-score / brain-score/vision

Wrapper identifiers are unconstrained, so distinct models can share an activation cache key

Open
#2,513 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
193
Forks
105
Avg merge
10h 48m
Merged PRs (30d)
10

Description

Follow-up to #2511, and a distinct defect from the one it fixes.

## The gap

The activation cache keys on `ActivationsExtractorHelper.self.identifier`, which comes from `PytorchWrapper(identifier=...)` — **not** from the registered model identifier. `model_registry` keys are unique by construction, and `ModelCommitment(identifier=...)` is unique, but nothing requires the wrapper's identifier to match either, or to be unique at all.

So registry uniqueness provides no protection: two separately registered models can share a cache key. @nilsleut's workaround note in #2503 points at the same mechanism — "the identifier has to be set on the `PytorchWrapper` itself, not only on the `ModelCommitment`".

## Current state

Scanning all model plugin `PytorchWrapper(identifier=...)` bindings, five identifiers are shared across plugin directories:

| wrapper identifier | dirs | plugins |
|---|---|---|
| `name` | 6 | `omnivore_swinB`, `resnet50_SIN`, `resnet50_SIN_IN`, `resnet50_SIN_IN_IN`, `resnet50_barlow`, `resnet50_vicreg` |
| `my-model` | 5 | `effnetb1_272x240`, `effnetb1_cutmix_augmix_sam_e1_5avg_424x377`, `effnetb1_cutmixpatch_SAM_robust32_avge6e8e9e10_manylayers_324x288`, `effnetb1_cutmixpatch_augmix_robust32_avge4e7_manylayers_324x288`, `resnet50_finetune_cutmix_AVGe2e3_robust_linf8255_e0_247x234` |
| `resnet50` | 5 | `r50_tvpt`, `resnet50_eMMCR_Vanilla`, `resnet50_eMMCR_VanillaV2`, `resnet50_eMMCR_eqp10_lm1`, `resnet_50_robust` |
| `alexnet` | 2 | `alexnet`, `alexnet_ks_torevert` |
| `sam_test_resnet` | 2 | `sam_test_resnet`, `sam_test_resnet_4` |

## Impact today: none (for now)

- `name`, `my-model` and `resnet50` are not registered model identifiers, so under revisioning they resolve to nothing and #2487's refusal path applies. They never collide — but they also never cache, so those 16 plugins recompute activations on every run.
- `alexnet`/`alexnet_ks_torevert` and `sam_test_resnet`/`sam_test_resnet_4` do share a resolved key, but each pair's `model.py` is byte-identical, so the shared activations are genuinely the same computation.

#2511 removes the resolution dimension of this, but not the general case: two plugins with different weights and identical preprocessing would still share a key.

## Suggested fix

Assert at plugin-test time that a plugin's wrapper identifier matches the identifier it registers. That closes the collision, and as a side effect makes the 16 currently-uncacheable plugins cacheable, since a registered identifier resolves a revision where `my-model` cannot.

Prior art for the shape of the guard: `brainscore_vision/benchmark_helpers/cache_contract.py`, added in #2489 for the analogous stimulus-identifier contract.

## Related

- #2503 / #2511 — preprocessing resolution in the cache key
- #2487 — refusing to cache when no plugin revision resolves
- #2489 — the stimulus-identifier contract and its repo-wide guard

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with brainscore_vision/benchmark_helpers/cache_contract.py and trace the plugin-test guard pattern to the PytorchWrapper and ModelCommitment identifiers. Scan the model plugin wrapper bindings listed in the issue, then add the plugin-time assertion. Done means each wrapper identifier matches its registered identifier and the existing shared or unresolvable identifiers are addressed by the guard.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.