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

Add torch API usage example #99

Merged
merged 13 commits into from
Dec 7, 2023

Conversation

timonmerk
Copy link
Contributor

Addresses #97

Added minimal example of torch API.

In comparison to the get_emissions function in the https://cebra.ai/docs/demo_notebooks/Demo_Allen.html notebook, I used the solver.transform function, to keep the example a bit shorter

@cla-bot
Copy link

cla-bot bot commented Oct 29, 2023

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.

@timonmerk
Copy link
Contributor Author

@cla-bot check

@cla-bot cla-bot bot added the CLA signed label Oct 29, 2023
@cla-bot
Copy link

cla-bot bot commented Oct 29, 2023

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.

Copy link
Member

@stes stes left a comment

Choose a reason for hiding this comment

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

Thanks @timonmerk for the contribution! I left some small comments to adapt the style of the example to the remainder of the code base and also started the test suite (the code in the usage.rst is tested as well).

docs/source/usage.rst Outdated Show resolved Hide resolved
docs/source/usage.rst Outdated Show resolved Hide resolved
docs/source/usage.rst Outdated Show resolved Hide resolved
timonmerk and others added 4 commits October 29, 2023 17:06
Co-authored-by: Steffen Schneider <steffen@bethgelab.org>
Co-authored-by: Steffen Schneider <steffen@bethgelab.org>
@MMathisLab MMathisLab added the documentation Improvements or additions to documentation label Oct 29, 2023
docs/source/usage.rst Outdated Show resolved Hide resolved
Copy link
Member

@stes stes left a comment

Choose a reason for hiding this comment

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

lgtm now, requesting review also from @MMathisLab

@stes stes requested a review from MMathisLab October 29, 2023 18:08
Copy link
Member

@MMathisLab MMathisLab left a comment

Choose a reason for hiding this comment

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

Looks good! I'd make Cebra ~> CEBRA and need to check why the tests fail :)

- minor typesetting
stes
stes previously requested changes Nov 15, 2023
Copy link
Member

@stes stes left a comment

Choose a reason for hiding this comment

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

actually, just noticed a few additional minor edits re the use of the __len__ functions... will edit

Copy link
Member

@stes stes left a comment

Choose a reason for hiding this comment

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

added a few comments to improve the example. apologies for missing these earlier. @timonmerk , no action needed here, I can do the edits

docs/source/usage.rst Outdated Show resolved Hide resolved
docs/source/usage.rst Show resolved Hide resolved
docs/source/usage.rst Show resolved Hide resolved
@MMathisLab
Copy link
Member

@stes okay to merge, and can be updated based on updates from @gonlairo later

@MMathisLab MMathisLab self-requested a review November 30, 2023 21:58
@MMathisLab
Copy link
Member

@stes I think this should be merged, and if you have improvements and input from @gonlairo we can add them, but nothing seems technically wrong, and it's been open >30 days, so I'd like to merge.

@MMathisLab MMathisLab requested a review from stes December 7, 2023 19:00
@MMathisLab MMathisLab dismissed stes’s stale review December 7, 2023 19:01

can be addressed in another PR

@MMathisLab MMathisLab merged commit 1438bb0 into AdaptiveMotorControlLab:main Dec 7, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA signed documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants