Skip to content
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

Brainstorm: Apple silicon support #663

Merged
merged 7 commits into from
Dec 11, 2023

Conversation

Edouard2laire
Copy link
Contributor

@Edouard2laire Edouard2laire commented Dec 5, 2023

This PR is here to use the maint branch of SPM when using an ARM Mac (which requires new Mex files) and fix a small bug

Note: this cannot be merged as is as brainstorm doesn't really like that SPM is in the folder called spm12-maint. But this works after adding SPM manually to the path: addpath('/Users/edelaire1/.brainstorm/plugins/spm12/spm12-maint')

Edit: actually would that be ok with you if we directly pull from the main branch of SPM: https://github.com/spm/spm ?

@Edouard2laire Edouard2laire changed the title Spm ARM SPM12 ARM version Dec 5, 2023
@rcassani
Copy link
Member

rcassani commented Dec 5, 2023

Since the only difference between the main (the download link) and maint branches are the *.mexmaca64 files. It would make more sense to change the plugin to point to maint It should work by changing PlugDesc(end).Version from latest to github-maint and using the maint archive ULR.

@Edouard2laire
Copy link
Contributor Author

Edouard2laire commented Dec 5, 2023

Like that or would you change the version for everyone? I guess most users don't need the new mex.

actually shoudn't we remove the mex from https://github.com/brainstorm-tools/brainstorm3/tree/master/external/spm ?
as they are included in spm plugin ?

@rcassani
Copy link
Member

rcassani commented Dec 6, 2023

The switch option should be ok. Not only saves the 6 MB for the Apple silicon binaries, but also saves the user of downloading from GitHub which can be slow, SPM is around 200 MB. I hope the zip in their page get updated at some point.

Regarding the duplicated binaries, the opposite approach will be done (later push by me also in this PR):

  1. Add the to external also the Apple silicon ones
  2. Remove them from the installed SPM as the binaries in external are the ones that add the In-Brainstorm MNI normalization (maff) method 😉

P.S. Adding the small bugfix for not existent plugin process directly in master

@Edouard2laire
Copy link
Contributor Author

Edouard2laire commented Dec 7, 2023

Thanks. let me know if i can help.

Maybe we should identify the toolbox that doesn't have new mex yet like iso2mesh, Mcxlab, and put a warning or something indicating that they cannot be used on ARM mac with matlab >2023b.

@rcassani
Copy link
Member

rcassani commented Dec 7, 2023

These things are needed:

  • A warning in the startup to indicate that Brainstorm is running on Apple silicon and some stuff may not work as expected, and recommend to use the Intel version.
  • Add the Apple silicon binaries for external SPM
  • Add warnings in the plugins that do not have support yet

Edit: It is quite a lot what is not supported yet:
https://www.mathworks.com/support/requirements/apple-silicon.html

@rcassani rcassani changed the title SPM12 ARM version Brainstorm: Apple silicon support Dec 7, 2023
@Edouard2laire
Copy link
Contributor Author

Edouard2laire commented Dec 11, 2023

Edit: It is quite a lot what is not supported yet:
https://www.mathworks.com/support/requirements/apple-silicon.html

it might seem like a lot but this is actually not really impacting us as its mainly packages for Simulink or specific decide like Arduino boards.

Here is an incomplete list of plugins that are not compatible yet

Maybe the generic warning is enough

@rcassani
Copy link
Member

rcassani commented Dec 11, 2023

11-dec-2023: This is what I have checked so far. Parenthesis indicate not supported due to dependency

❓ 'adi-sdk'
✅ 'axion'
❓ 'bci2000'
✅ 'blackrock'
❌ 'brain2mesh' (iso2mesh)
✅ 'brainentropy'
✅ 'cat12'
❌ 'ct2mrireg'
✅ 'derivelfp'
❌ 'duneuro'
✅ 'easyh5'
✅ 'fastica'
❌ 'fieldtrip'
❌ 'iso2mesh'
✅ 'jsnirfy'
❓ 'kilosort'
❓ 'kilosort-wrapper'
❓ 'libsvm'
❌ 'mcxlab-cl'
❌ 'mcxlab-cuda'
❓ 'mff'
✅ 'mia'
✅ 'neuroelectrics'
✅ 'neuromaps'
✅ 'nirstorm'
✅ 'npy-matlab'
❓ 'nwb'
❌ 'openmeeg'
❓ 'phy' (?)
✅ 'picard'
❌ 'plexon'
❓ 'plotly'
❌ 'roast' (iso2mesh)
❌ 'simmeeg' (fieldtrip)
✅ 'spm12'
❓ 'tdt-sdk'
✅ 'ultramegasort2000'
❌ 'waveclus'
❓ 'xdf'

@Edouard2laire
Copy link
Contributor Author

Edouard2laire commented Dec 11, 2023

For brainentropy, if fminunc is available, then it's already working as part of the optimisation toolbox.

Edit; in case its needed, I recompiled it: multifunkim/best-brainstorm#15

@rcassani
Copy link
Member

Added an error message for the Plugins that have been already tested.

As testing exhaustively all plugins may take a long time, due to lack of HW to test on my side. I think we should merge this PR for the moment and update the list (in f72d0a6) with future tests and reports from users. What do you think?

@Edouard2laire
Copy link
Contributor Author

Edouard2laire commented Dec 11, 2023

Also added an error when loading an already downloaded plugin. I'll try to compile them slowly first with iso2mesh and mxclab.

I agree that we can merge it like that.

@rcassani rcassani merged commit f30c905 into brainstorm-tools:master Dec 11, 2023
@Edouard2laire Edouard2laire deleted the SPM-ARM branch December 11, 2023 21:59
rcassani added a commit that referenced this pull request Dec 12, 2023
@Edouard2laire
Copy link
Contributor Author

update. I was able to compile iso2mesh today: fangq/iso2mesh#75

Some tests are needed but when this is merged; we should be able to check that from our todo list :)

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.

2 participants