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

ImportError Fixed: Use jax.config.update and remove import .__version #138

Merged
merged 2 commits into from
Jul 20, 2024

Conversation

stevengogogo
Copy link
Contributor

@stevengogogo stevengogogo commented Jul 15, 2024

Hello Nico and contributors of tornadox,

Thanks for sharing your work!

When importing your package, I found two issues that cause import error: (1) Importing jax.config is deprecated in newer Jax (2) ._version file is missing. This PR provides a minimal change that required to import with latest Jax version. Please let me know your thoughts!

Issue

The changes of this PR include:

Importing jax.config

Since JAX 0.4.20, the following way to import config is deprecated. This can cause error for newer Jax version, and the current version fails to import

>>> import tornadox
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/stevenchiu/miniconda3/envs/numjax/lib/python3.11/site-packages/tornadox/__init__.py", line 3, in <module>
    from jax.config import config
ModuleNotFoundError: No module named 'jax.config'

._version not found

Another issue is the ._version is missing in the package. I simply removed this part to make the package imported without error.

Suggestion

As described in this PR, I did two thing in __init__.py

  1. Change the way to import jax.config
  2. Remove importing ._version

Tested System

jax                       0.4.30                   pypi_0    pypi
jaxlib                    0.4.30                   pypi_0    pypi

Best,
Steven Chiu

@stevengogogo stevengogogo marked this pull request as ready for review July 15, 2024 21:53
@pnkraemer
Copy link
Owner

pnkraemer commented Jul 17, 2024

Hi, thanks for reaching out!

We don't really work with Tornadox anymore. (We should probably make a note somewhere...).

Instead, I recommend you check out Probdiffeq (https://github.com/pnkraemer/probdiffeq) for JAX code or ProbNumDiffEq.jl (https://github.com/nathanaelbosch/ProbNumDiffEq.jl) for Julia code. These two are more actively maintained. On the JAX side, Probdiffeq can do everything that Tornadox does (and more).

That said, if you would like to work with Tornadox specifically, we can look into this. What do you think? :)


Edit: in addition to everything above, the jax.config import change would be helpful! But the version import confuses me slightly. When did you see this error? After installing via pip install or in your fork?

@stevengogogo
Copy link
Contributor Author

Edit: in addition to everything above, the jax.config import change would be helpful! But the version import confuses me slightly. When did you see this error? After installing via pip install or in your fork?

I used Poetry install, but I agree the verions import might be related to local setup.

I'll look into probdiffeq. For now, I think tornadox provides simple code to learn.

@pnkraemer
Copy link
Owner

I used Poetry install, but I agree the verions import might be related to local setup.

I agree that it might be a setup issue - the tests pass, which is a great indicator. Thanks for reverting this one change!

I'll look into probdiffeq. For now, I think tornadox provides simple code to learn.

Awesome. I'd love to hear your thoughts once you've played around with these repos a bit :)

Copy link
Owner

@pnkraemer pnkraemer 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 to me.

The linting-related CI fails, but that is unrelated to this PR. I will merge this now.

Thanks for contributing!

@pnkraemer pnkraemer merged commit e15eb9d into pnkraemer:main Jul 20, 2024
1 of 2 checks passed
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.

2 participants