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

use brax.v1 and update requirements #156

Merged
merged 9 commits into from
Oct 9, 2023

Conversation

DBraun
Copy link
Contributor

@DBraun DBraun commented Oct 3, 2023

Related issues: #155

Where necessary I've changed brax to brax.v1. This is a short-term solution because v1 is already deprecated. However, it at least gets QDax working with jax/jaxlib 0.4.16 and flax 0.7.4. At the moment, if you install QDax from scratch without previously installing jax/flax, you end up with an unusable flax/qdax. This fixes that.

This PR is really just a starting point for discussion. I have installed it locally and imported various QDax things which work fine.

Checks

  • a clear description of the PR has been added
  • sufficient tests have been written
  • relevant section added to the documentation
  • example notebook added to the repo
  • clean docstrings and comments have been written
  • if any issue/observation has been discovered, a new issue has been opened

Future improvements

In the future QDax should not use brax.v1. Someone could make this a new issue soon.

@DBraun
Copy link
Contributor Author

DBraun commented Oct 5, 2023

Is any QDax maintainer able to help move this forward?

@Lookatator Lookatator self-requested a review October 5, 2023 13:32
@Lookatator
Copy link
Member

Is any QDax maintainer able to help move this forward?

Hi @DBraun,
Thanks very much for this PR, I'll review it either today or tomorrow 😄

@Lookatator Lookatator changed the base branch from main to develop October 9, 2023 09:54
@Lookatator Lookatator self-requested a review October 9, 2023 12:01
Copy link
Member

@Lookatator Lookatator left a comment

Choose a reason for hiding this comment

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

Seems good to me overall!

To run the PR tests and make it possible to compile the docs and run the automated pipeline, could you:

  1. merge develop into your branch?
  2. and change the version of python in the .readthedocs.yaml to 3.9 ?

requirements.txt Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Merging #156 (98c5999) into develop (c111cea) will not change coverage.
The diff coverage is 93.61%.

❗ Current head 98c5999 differs from pull request most recent head 7dd286c. Consider uploading reports for the commit 7dd286c to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@           Coverage Diff            @@
##           develop     #156   +/-   ##
========================================
  Coverage    92.23%   92.23%           
========================================
  Files          120      120           
  Lines         6992     6992           
========================================
  Hits          6449     6449           
  Misses         543      543           
Files Coverage Δ
qdax/__init__.py 100.00% <100.00%> (ø)
qdax/environments/base_wrappers.py 82.92% <100.00%> (ø)
qdax/environments/exploration_wrappers.py 32.97% <100.00%> (ø)
qdax/environments/init_state_wrapper.py 90.00% <100.00%> (ø)
qdax/environments/locomotion_wrappers.py 85.49% <100.00%> (ø)
qdax/environments/pointmaze.py 97.22% <100.00%> (ø)
qdax/environments/wrappers.py 96.96% <100.00%> (ø)
qdax/environments/humanoidtrap.py 18.30% <87.50%> (ø)
qdax/environments/__init__.py 86.79% <80.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Lookatator Lookatator merged commit df954c2 into adaptive-intelligent-robotics:develop Oct 9, 2023
5 checks passed
@DBraun DBraun deleted the brax-v1 branch January 26, 2024 23:18
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.

None yet

3 participants