Repository navigation
feat(contract): unify subjects and preserve legacy benchmarks - #201
Merged
Merged
Conversation
Compatibility is now one rule for every subject, read from declarations only: requested outputs are produced, at least one benchmark input is taken in, and the subject's required inputs are sent. A subject that takes in only some inputs runs; check_compatibility returns a CompatibilityMatch (full or partial, consumed and dropped inputs) so the score can record it. Removes the isinstance branch between native and typed subjects. Undocumented channels now fail pre-flight instead of warning.
UnifiedModel now runs sessions itself through its typed paths, so every subject reaches evaluation through interact and the helpers no longer check which class defined interact. Outputs are declared, not found: UnifiedModel reports neural channels from its region map and stops reading private attribute names; BrainScoreModel adds behavior and motor from its own configuration.
Wrong directions, bad addresses and failed family validators raise at construction instead of at the first event. Unregistered families and documented ndim/dtype hints only warn, since exploratory experiments use them and scoring already rejects unregistered channels at pre-flight.
LegacyBenchmarkFacade implements look_at, visual_degrees, digest_text, start_neural_recording and start_behavioral_task over a typed subject; BrainScoreModel keeps them as one-line forwards. Fidelity fixes: several requested time bins now repeat a static response as legacy vision models do (they were stored and ignored); a request for trial variance and the legacy language behavioral tasks fail with a clear error instead of an AttributeError or identical trials.
The built-in parts (region map, readout or answer, policy, intervention) are now session capabilities registered like added ones. interact, check_session_support and the declared channels all read that registry, replacing the if-chain beside it. Added capabilities still win by sorting first. An intervention now declares the lesion and stimulation inputs and the perturbation output it already handled; the perturbation family is added to the catalog.
extractor, answer, policy and intervention are the names; preprocessors, activations_model, generation_fn, action_fn and state_change_fn keep working with a deprecation warning. Extractor documents the one activity-recording role (input preparation, forward pass, hooks, caching, packaging) as a protocol nothing has to subclass.
The neural session driver no longer checks hasattr(subject, '_process_multi_modality'); BrainScoreModel's neural session handler says it combines inputs, other typed models are called without the keyword. Removes six BrainScoreModel delegators nothing used.
LegacyBenchmarkFacade records each legacy setting and presents the stimuli as a BufferedSession carrying time bins, trial count, task and recording type, then runs it through the subject's interact. Any subject takes part the same way: BrainScoreModel, the domain adapters and interact-only subjects. digest_text returns every configured output from one call. mark_legacy_benchmark and subject_for_benchmark let the domain loaders apply the facade only to benchmarks that call the legacy methods. The typed neural driver refuses require_variance instead of returning identical trials. BrainScoreModel seeds the facade from its own recording and task state, so a direct look_at after start_task sends the active task again; that refits the readout on cached features and leaves the result unchanged.
interact wraps each session in DeclaredInputSession with the model's in_channels. Events of undeclared channel families are skipped and a stimulus table loses the columns of undeclared modalities before any part sees it, so a stream the subject cannot take in never enters it (rule 5.5). The table keeps its identity, and therefore its cache key, when nothing is dropped. Typed environment steps pass through unchanged.
A region map names where to record; only an extractor can record there. NeuralSessions is enabled only when both exist, so a BrainScoreModel built without an extractor no longer declares neural:<region> outputs it cannot produce (rule 2.4). The motor test gains the extractor its neural assertion relied on.
process() is no longer abstract. The default interact still drives a typed model through process, and a subclass without one fails there with a message naming both options. Closes the rule 1.1/1.3 row of the known gaps: the typed base no longer forces a named evaluation method.
…n start Every seeded channel family now carries a payload_type (paths or arrays for sensory inputs, str for text and instructions, StateChange for interventions, EnvironmentStep for observations, assemblies for neural and behavioral outputs, EnvironmentResponse and PerturbationApplied for the rest). BufferedSession reports a mismatch when it validates its inputs. Warn-first: the mismatch is a note next to the unregistered-family and ndim/dtype notes, and validate() is unchanged, so no session that runs today is rejected. Promote the check to an error once the notebooks and examples have been reviewed against the declared types.
…rations The pre-flight probe no longer asks whether the subject has start_recording or which class defined it. It reads the neural output channels the benchmark requests or the subject declares, keeps the region map order when there is one, and runs one stimulus as a session through interact, the way every subject is evaluated. A subject that declares no neural output has nothing to probe and the guard stays off, as documented. The test fakes become real typed subjects over real stimulus tables that answer with assemblies; the duck-typed objects they replaced only worked because the probe called process directly.
…recated names The dispatcher, the behavioral readout and the built-in capabilities read recording, task and perturbation state from the component that owns it (_recorder, _behavioral, _perturbations) instead of through the private property bridges on BrainScoreModel. The bridges stay as a compatibility surface for tests and notebooks and are marked as such. The deprecation warnings for the old constructor names and for extraction callbacks in preprocessors now say when they go away: at the public release, together with the interact-to-participate rename.
…itting inputs, refuse behavioral variance Addresses the four findings of the 2026-11-01 EC2 review of this branch. The facade sends one TaskContext object with every measurement and puts the per-measurement settings (trial count, variance request, recording type, legacy text) on the session as `settings`, beside the task instead of merged into its metadata. Keyword settings a benchmark passes to start_task (Malania2007's number_of_trials and require_variance) travel as metadata['task_settings'], so fitting and measurement configuration never overwrite each other. BrainScoreModel.start_task recognizes the task object it is already configured for and keeps the fitted readout, so a facade session that re-sends the task per measurement no longer refits; a new context, or any context after a state change was applied or undone, fits again. Fitting stimuli reach the readout with only the declared input modalities, as a new table that keeps the identifier; the benchmark's table is untouched. The behavioral typed driver refuses require_variance like the neural one. Legacy text identifiers hash length-prefixed parts so different segmentations of the same characters never share a cache entry.
The coordinated workflow reads peer revisions from the pinned unified commit, so it now points at the unified revision whose install/peer-revisions.json names the facade branch tips; the requirements pin follows the same core revision. Re-pin to merged revisions before merging.
TaskContext is a public mutable dataclass. The identity memo that keeps the fitted readout for a task object sent again now also compares what the fit depends on: task type, the fitting table and its rows and labels, and the generation settings. Swapping the fitting table or relabelling it in place and calling start_task again fits anew; sending the unchanged task with each measurement still does not.
The per-measurement settings a legacy facade attaches to a session (trial count, variance request, recording type, legacy text) are a Session context field with a None default, so recorders that save Session.context_fields keep them and replay restores them. A replayed measurement asks the subject for the same trial count it did originally.
This was referenced Nov 2, 2026
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.
Run existing benchmarks through the same session contract used by new UMI experiments. This combines the selected Contract v3 simplification and the reviewed permanent legacy-benchmark facade; it supersedes #199 and #200.
Changes
Subject.interact(session)as the contract entry point;UnifiedModel.processis no longer abstract. Declare abilities through input, output and required channels.preprocessors=path remains deprecated.LegacyBenchmarkFacadefor legacy task/recording setup andlook_at/digest_text. Each measurement becomes aBufferedSessionrun throughinteract, preserving time bins, task keywords, trial counts and recording settings. The facade is permanent.session.settingsto recorded/replayed context. Preserve single-channel payload identity and honor multiple time bins.interact, without subject class checks or capability discovery. Read component state directly; retain compatibility property bridges.validate()behavior remains unchanged.Behavior to review
Unknown
look_atsettings raiseTypeError. DirectBrainScoreModel.look_atwithmultiple recording regions raises; typed multi-region helpers remain available.
visual_degrees()is read publicly where needed; no default is invented.The
interacttoparticipaterename, builder naming decision, strict payloadrejection and score-sensitive registration rewrites remain separate changes.
Coordination
Merge core first. The domain/unified PRs must then pin the actual merged core
revision and rerun coordinated CI. This candidate's final core commit changes CI
pins only; its qualified runtime revision is
15dcc3c.Qualification
Independent Linux EC2 qualification used Python 3.11, numpy 1.26.4,
xarray 2022.3.0, scikit-learn 1.7.2, transformers 4.57.6 and torch 2.6.0.
Pereira parity covers historical linear variants, not published ridge qualification.
Production-reference differences also occur on clean v2; no historical cause is inferred.
Accepted exclusion: Kartik closed this scoped review with Malania excluded.
Direct base, direct candidate and facade response arrays match exactly, but named
coordinate fidelity remains unverified. All three return a low-accuracy threshold-fit
fallback, not a valid fitted threshold. Malania is excluded, not counted as passed.
Full report and numerical results.
This qualification does not complete the broader release matrix.
Coordinated PRs