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 Classic Control Gymnax #90

Open
wants to merge 39 commits into
base: development
Choose a base branch
from
Open

Add Classic Control Gymnax #90

wants to merge 39 commits into from

Conversation

benjamc
Copy link
Collaborator

@benjamc benjamc commented Jun 7, 2023

No description provided.

@benjamc benjamc marked this pull request as ready for review June 7, 2023 13:06
@benjamc
Copy link
Collaborator Author

benjamc commented Jul 3, 2023

Pause until new CARL env was integrated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why call them Jax and not Gymnax? I bet there are other attempts at Jaxing them, Gymnax would be more explicit imo

Copy link
Contributor

Choose a reason for hiding this comment

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

Env params is frozen for a reason, but I guess this runs, right? Pretty sure we can't jit it, but we have this issue with brax as well... at some point we might want to brainstorm if we can somehown avoid replacing env parts to change context. For now it's alright, though.

Copy link
Contributor

@TheEimer TheEimer 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! One question though: have you tested if they're actually faster? In brax our context changes kind of break the speed, does it also happen here?

Copy link

codecov bot commented Dec 11, 2023

Codecov Report

Merging #90 (289d324) into development (f30d834) will increase coverage by 29.05%.
Report is 796 commits behind head on development.
The diff coverage is 49.43%.

❗ Current head 289d324 differs from pull request most recent head 7c2e21e. Consider uploading reports for the commit 7c2e21e to get more accurate results

Additional details and impacted files
@@               Coverage Diff                @@
##           development      #90       +/-   ##
================================================
+ Coverage        19.05%   48.10%   +29.05%     
================================================
  Files               78       84        +6     
  Lines             4030     4122       +92     
  Branches           555      543       -12     
================================================
+ Hits               768     1983     +1215     
+ Misses            3224     2060     -1164     
- Partials            38       79       +41     

Impacted file tree graph

@benjamc
Copy link
Collaborator Author

benjamc commented Jan 8, 2024

  • Jaxify CARLEnv:

CARLJaxEnv:

  • env state: sub env state + context id
  • env params: empty
  • gymnax environment api

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