Skip to content

Let BaRISTA encode recordings with different montages - #1175

Merged
bruAristimunha merged 3 commits into
braindecode:reopen-1171-baristafrom
julien-gadonneix:barista-variable-montage
Sep 21, 2026
Merged

bruAristimunha merged 3 commits into
braindecode:reopen-1171-baristafrom
julien-gadonneix:barista-variable-montage

Conversation

@julien-gadonneix

Copy link
Copy Markdown
Collaborator

Follow-up to #1173. BaRISTA is currently locked to the montage and window length it was built with; this makes the montage a forward argument so a single model can encode recordings from different subjects.

  • forward(x, spatial_indices=None, return_features=False): spatial_indices is the montage of this batch, (n_chans, 3) for "coords" and (n_chans,) for the region scales. Omitting it falls back to the montage resolved at construction, so model(X) behaves as before.
  • The embedding tables only depend on the spatial scale, so the model can now be built without any index. Constructor indices and chs_info positions become the default montage instead of a requirement, and the "no spatial indices" error moves to forward.
  • Rotary angles are computed per call from an inv_freq buffer instead of being baked into fixed cos/sin buffers (numerically identical on the fixed grid).
  • The channel-count and window-length checks in forward are dropped: pooling="mean" accepts any montage and window length, while pooling="learned" keeps its fixed token count and its error now points at pooling="mean".

All BaRISTA tests pass, including the torch.compile, torch.export and TorchScript cases.

Copilot AI lite review requested due to automatic review settings September 21, 2026 19:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings September 21, 2026 19:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings September 21, 2026 19:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@bruAristimunha
bruAristimunha merged commit 1f35ecc into braindecode:reopen-1171-barista Sep 21, 2026
10 of 11 checks passed
@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.95238% with 8 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (reopen-1171-barista@ca18fd1). Learn more about missing BASE report.

Additional details and impacted files
@@                  Coverage Diff                   @@
##             reopen-1171-barista    #1175   +/-   ##
======================================================
  Coverage                       ?   86.81%           
======================================================
  Files                          ?      143           
  Lines                          ?    16482           
  Branches                       ?        0           
======================================================
  Hits                           ?    14309           
  Misses                         ?     2173           
  Partials                       ?        0           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@julien-gadonneix
julien-gadonneix deleted the barista-variable-montage branch September 21, 2026 21:14
bruAristimunha added a commit that referenced this pull request Sep 29, 2026
* Restore BaRISTA model for review (#1171)

* Simplify BaRISTA and fix model contract edge cases

* Read BaRISTA spatial indices from dataset metadata

* Preserve shared class targets with unique MNE event IDs

* Link BaRISTA release note to the restored PR

* changes to allow for different n_chans (#1175)

Co-authored-by: Bru <[email protected]>

* Simplify BaRISTA spatial setup and license note

* Add BaRISTA figure and verify conversion of released weights

* Publish the converted BaRISTA encoders to the Hub

Make the converter the single recipe that ships with the weights: it now
writes a model card and copies itself and NOTICE.txt into each Hub
directory, and --push-to uploads them through BaRISTA.push_to_hub.

The published encoders pool by mean so one file serves any montage and
window length, and the class docstring points at the three repositories.

* Publish BaRISTA encoders without the untrained head

Co-authored-by: Cursor <[email protected]>

* fix: validate BaRISTA forward indices and align the coordinate fallback

- Move spatial indices passed to forward onto the embedding device and
  range-check them in eager mode (clear ValueError instead of IndexError;
  export/TorchScript/compile paths unchanged).
- Reorder the MNE coordinate fallback to the (left, inferior, posterior)
  columns of the released tables; negated RAS alone gave (L, P, I).
- Drop the converter's --pooling learned option, which saved a randomly
  initialised read-out: the releases carry no pooling weights.
- Docstring opening follows the "<Name> from <Author> et al" convention.
- Add test_barista.py: region scales, per-forward montages, invalid
  indices, the learned-pooling grid check and the coordinate order.

* BaRISTA: keep the conversion script on the Hub, link the license

The converter now lives only in the braindecode/BaRISTA-{coords,parcels,lobes}
Hub repositories (synced with this PR's version); the docstring points there
and the license note is a single link.

* Keep BaRISTA notice concise with upstream license link

* Integrate BaRISTA coverage into existing model tests

---------

Co-authored-by: Julien GADONNEIX <[email protected]>
Co-authored-by: Cursor <[email protected]>
@bruAristimunha bruAristimunha mentioned this pull request Oct 5, 2026
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