search

AdaptiveMotorControlLab / CEBRA

Learnable latent embeddings for joint behavioral and neural analysis - Official implementation of CEBRA
https://cebra.ai
Other
875 stars 66 forks source link

Implementation for discrete multisession #135

Closed introspective-swallow closed 2 months ago

introspective-swallow commented 4 months ago

I implemented the discrete multisession mode for CEBRA, analogous to the continuous multisession mode. It works for one discrete label and uses the uniform distribution for prior sampling. I also included it in the sklearn integration. I added tests for the sampler, loader, solver and the sklearn integration.

Added objects that differ from the continuous multisession are the loader DiscreteMultiSessionDataLoader and the sampler DiscreteMultisessionSampler.

As an example, this are session embeddings fitted with discrete multisession for the data I am working with (that will be published soon):

multidisc

I am new to open source contribution, so I'm excited to get your corrections and feedback!

cla-bot[bot] commented 4 months ago

Thank you for your contribution. We require contributors to sign our Contributor License Agreement (CLA). We do not have a signed CLA on file for you. In order for us to review and merge your code, please sign our CLA here. After you signed, you can comment on this PR with @cla-bot check to trigger another check.

stes commented 4 months ago

Hi @introspective-swallow , thanks a lot for your contribution! I skimmed the code and it looks good overall, made two small comments. I just started to run tests. I'll do another in-depth pass through the code in the next days.

stes commented 4 months ago

Also, if you don't mind, do you have an example of a fitted embedding that you could add to the PR description as a reference?

stes commented 4 months ago

@introspective-swallow a few tests are failing, could you have a look? I created another PR #136 as some of the failing tests are unrelated to your contribution. I will fix these independently, but others are failing due to the test setup in this PR. Let me know if you have questions about the particular tests.

You can also run them locally using pytest!

introspective-swallow commented 4 months ago

Also, if you don't mind, do you have an example of a fitted embedding that you could add to the PR description as a reference?

I do have one but the data is private for now though it will be published soon. I could remove the label names and include a fitted embedding, would that work? After it is published we could share it to make a demo as well.

cla-bot[bot] commented 4 months ago

Thank you for your contribution. We require contributors to sign our Contributor License Agreement (CLA). We do not have a signed CLA on file for you. In order for us to review and merge your code, please sign our CLA here. After you signed, you can comment on this PR with @cla-bot check to trigger another check.

introspective-swallow commented 4 months ago

@introspective-swallow a few tests are failing, could you have a look? I created another PR #136 as some of the failing tests are unrelated to your contribution. I will fix these independently, but others are failing due to the test setup in this PR. Let me know if you have questions about the particular tests.

You can also run them locally using pytest!

Yes! I fixed and added some tests and now pytest is giving me no errors.

stes commented 4 months ago

Yes! I fixed and added some tests and now pytest is giving me no errors.

Perfect. In the meantime I finished #136 to resolve the upstream errors unrelated to this PR. I will get this merged, then update your PR, and confirm that all tests build.

I do have one but the data is private for now though it will be published soon. I could remove the label names and include a fitted embedding, would that work? After it is published we could share it to make a demo as well.

That sounds great, thanks a lot!

cla-bot[bot] commented 4 months ago

Thank you for your contribution. We require contributors to sign our Contributor License Agreement (CLA). We do not have a signed CLA on file for you. In order for us to review and merge your code, please sign our CLA here. After you signed, you can comment on this PR with @cla-bot check to trigger another check.

stes commented 4 months ago

Added the changes from #136 to see if tests pass now.

stes commented 4 months ago

Thank you for your contribution. We require contributors to sign our Contributor License Agreement (CLA). We do not have a signed CLA on file for you. In order for us to review and merge your code, please sign our CLA here. After you signed, you can comment on this PR with @cla-bot check to trigger another check.

@introspective-swallow could you also fill this form and re-run the CLA check?

cla-bot[bot] commented 4 months ago

Thank you for your contribution. We require contributors to sign our Contributor License Agreement (CLA). We do not have a signed CLA on file for you. In order for us to review and merge your code, please sign our CLA here. After you signed, you can comment on this PR with @cla-bot check to trigger another check.

cla-bot[bot] commented 4 months ago

Thanks for tagging me. I looked for a signed form under your signature again, and updated the status on this PR. If the check was successful, no further action is needed. If the check was unsuccessful, please see the instructions in my first comment.

stes commented 4 months ago

Ok nice, tests look good. Will do another pass through the code in the next days.

introspective-swallow commented 4 months ago

Thank you for your contribution. We require contributors to sign our Contributor License Agreement (CLA). We do not have a signed CLA on file for you. In order for us to review and merge your code, please sign our CLA here. After you signed, you can comment on this PR with @cla-bot check to trigger another check.

@introspective-swallow could you also fill this form and re-run the CLA check?

@cla-bot check Yes, here

cla-bot[bot] commented 4 months ago

Thank you for your contribution. We require contributors to sign our Contributor License Agreement (CLA). We do not have a signed CLA on file for you. In order for us to review and merge your code, please sign our CLA here. After you signed, you can comment on this PR with @cla-bot check to trigger another check.

cla-bot[bot] commented 4 months ago

Thanks for tagging me. I looked for a signed form under your signature again, and updated the status on this PR. If the check was successful, no further action is needed. If the check was unsuccessful, please see the instructions in my first comment.

introspective-swallow commented 4 months ago

@cla-bot check

cla-bot[bot] commented 4 months ago

Thank you for your contribution. We require contributors to sign our Contributor License Agreement (CLA). We do not have a signed CLA on file for you. In order for us to review and merge your code, please sign our CLA here. After you signed, you can comment on this PR with @cla-bot check to trigger another check.

cla-bot[bot] commented 4 months ago

Thanks for tagging me. I looked for a signed form under your signature again, and updated the status on this PR. If the check was successful, no further action is needed. If the check was unsuccessful, please see the instructions in my first comment.

introspective-swallow commented 4 months ago

@stes I already filled the CLA and after triggering a check it doesn't seem to work :(

stes commented 4 months ago

@introspective-swallow just checked, you filled in the URL to your full github username (https://github.com/introspective-swallow). you need to correct this to only your username, then it will work.

stes commented 4 months ago

@cla-bot check

cla-bot[bot] commented 4 months ago

Thanks for tagging me. I looked for a signed form under your signature again, and updated the status on this PR. If the check was successful, no further action is needed. If the check was unsuccessful, please see the instructions in my first comment.

stes commented 4 months ago

fixed! cc @introspective-swallow

introspective-swallow commented 4 months ago

Thank you! My mistake.

stes commented 4 months ago

No worries!

MMathisLab commented 2 months ago

only last issue is to edit the docs to say how to use it. thanks again @introspective-swallow -- sorry for the delay in reviewing this!

MMathisLab commented 2 months ago

one minor doc issue to fix (I can do so tomorrow!):


_____________________________ [doctest] usage.rst ______________________________
1039                                                       num_batches=5)
1040 
1041     # multi-session
1042     multi_score = cebra.sklearn.metrics.infonce_loss(multi_cebra_model,
1043                                                      neural_session1,
1044                                                      continuous_label1,
1045                                                      session_id=0,
1046                                                      num_batches=5)
1047 
1048 
UNEXPECTED EXCEPTION: ValueError('Labels invalid: must have the same type of features as the ones used for fitting,expects int64, got float64.')
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/pytest_sphinx.py", line 366, in _DocTestRunner__run
    exec(
  File "<doctest usage.rst[33]>", line 9, in <module>
  File "/home/runner/work/CEBRA/CEBRA/cebra/integrations/sklearn/metrics.py", line 87, in infonce_loss
    cebra_model._check_labels_types(y, session_id=session_id)
  File "/home/runner/work/CEBRA/CEBRA/cebra/integrations/sklearn/cebra.py", line [86](https://github.com/AdaptiveMotorControlLab/CEBRA/actions/runs/9258095298/job/25467946597?pr=135#step:10:87)3, in _check_labels_types
    raise ValueError(
ValueError: Labels invalid: must have the same type of features as the ones used for fitting,expects int64, got float64.
usage.rst:1048: UnexpectedException
MMathisLab commented 2 months ago

will will fix the rest of the issues in this issue, as it's related to matplotlib and not this PR: https://github.com/AdaptiveMotorControlLab/CEBRA/pull/150