-
Notifications
You must be signed in to change notification settings - Fork 4
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
Repo reorganization #29
Comments
Another Python RASR implementation just got published today: https://github.com/nbara/python-meegkit |
@jonathanjfshaw : thanks. There are some significant difference in design from the Matlab implementation here, so both version could be useful. @sylvchev : I think they struggling also with pymanopt because the nonlinear_eigenspace is implemented but not in use. |
Hi @lkorczowski , I wrote the other ASR python code above. Let me know if I can be of any help to compare/benchmark/test our respective implementations. The Matlab code is anything but straightforward, so the most important thing is that we get the expected results. |
@nbara : Great. Thanks. We still had some trouble finding the right metric to make a quantitative comparison. IMO, the best way would be to use hybrid EEG data (clean + artificial artifacts) but it will be bit of work to make implement this framework. Do you have more ideas? |
If you can already check that your implementation gives the same result as Matlab's visually, I think you're fine. I agree that some kind of quantitative test would be even better (because the original Matlab code could contain bugs as well after all). You may already have read it but maybe this paper will give you some ideas? |
The repo is nearly ready for publication. More cleanup is necessary, but it can wait after we have a PIP package:
|
At this point, we should clean and reorganize this repo before publishing it. We have a couple options: either we start a new repo from the timeflux_example template and update it, or we transfer this one to the timeflux organization. I don't have any preference, but if we start from a clean slate, we must be sure to create at least one new issue to reference the current problems and report the backlog for future reference.
For consistency, I would recommend the following structure:
I will update the
examples
directory with at least one usage example and a small annotated data file.estimation.py
should be renamed torasr.py
.In the
helpers
directory, please keep only what you actually use fromutils.py
and maybe rename the file to something more meaningful. We will later decide with @bertrandlalo what could be migrated to Timeflux core.Please make sure that the dependencies in
requirements.txt
andsetup.py
are satisfied. The file should contain the absolute minimum to make it run in a Timeflux pipeline. If other dependencies are needed for the notebooks (mne, matplotlib, etc.), write another dependency file.I am not sure to understand what purpose
test_config.py
andtest_viz
actually serve.The hardcoded paths in
utils/config.py
made me cry ;)The text was updated successfully, but these errors were encountered: