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

Adds interoperability with PyTensor (and PyMC) #621

Merged
merged 34 commits into from
Nov 20, 2024
Merged

Conversation

cako
Copy link
Collaborator

@cako cako commented Nov 17, 2024

Description

Adds the ability to call use PyLops Linear Operators as PyTensor Ops. This allows PyMC to use PyLops LinearOperators inside of their models.

The PR also includes an example of Bayesian linear regression (examples/plot_bayeslinearregr.py) demonstrating this capability.

Changes

In addition to the main functionality, this PR issues several fixes:

  • Ignore __pycache__
  • Disables html_theme_options not available for current theme
  • Various formatting issues in RST, math, etc.

Notes

Although I added everything to intersphinx, the documentation links is not appearing in my local documentation. Don't know if this is by design or if I need to change anything.

@cako cako added the enhancement New feature or request label Nov 17, 2024
@cako cako requested a review from mrava87 November 17, 2024 07:06
@cako
Copy link
Collaborator Author

cako commented Nov 17, 2024

@mrava87 I've been having a very hard time satisfying the version requirements of all libraries. PyTensor does not support numpy 2 (pymc-devs/pytensor#689). Devito only supports numpy 2 for the latest versions, so dropping to numpy 1.x forces one to use a very old version of devito... which in turns makes it not support newer versions of sympy (above 0.13). On the other hand, PyTorch requires versions of sympy>=0.13.1.

Errata: looks like devito does support numpy 1.x for the newest versions. I'll give this another try!

@cako
Copy link
Collaborator Author

cako commented Nov 17, 2024

@mrava87 I think I managed to build it, the trick was to prevent the pip command which install pytorch to overwrite the sympy installation. I did this setting pytorch version to be below 2.5.

Note that these version changes are only required for the requirements-dev.txt and requirements-torch.txt files because they are used separately when installing. They are not needed in the requirements-doc.txt or the yml files because the dependency managers have access to all packages at once and therefore are able to figure out that you need numpy<2 and pytorch<2.5 to be compatible with pytensor.

@cako
Copy link
Collaborator Author

cako commented Nov 18, 2024

@mrava87 managed to get everything working after a lot of back and forth with versions :)

https://pylops--621.org.readthedocs.build/en/621/gallery/plot_bayeslinearregr.html#sphx-glr-gallery-plot-bayeslinearregr-py

@mrava87
Copy link
Collaborator

mrava87 commented Nov 18, 2024

Great! I’ll take a look in the next couple of days 😀

Copy link
Collaborator

@mrava87 mrava87 left a comment

Choose a reason for hiding this comment

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

This looks great 🚀

I just left a few minor comments... I am happy for this to be merged as soon as you feel it's ready 😄

examples/plot_bayeslinearregr.py Outdated Show resolved Hide resolved
requirements-doc.txt Show resolved Hide resolved
@cako
Copy link
Collaborator Author

cako commented Nov 19, 2024

@mrava87 I actually tested the yml file as it current stands, it didnt install the correct numpy. The only way I could get it to work was to version pin:

- numpy>=1.21.0,<2
- scipy>=1.11.0,<1.13
- pytorch>=1.2.0,<2.5
 - pip:
  - devito==4.8.6

I'm reluctant to add this to the "standard" environment-dev.yml since that would force everyone to use old versions just so that pytensor still works. I think instead I will remove pytensor from the yml files, and put a check to not execute the pytensor tests if numpy>=2. What do you think?

@mrava87
Copy link
Collaborator

mrava87 commented Nov 19, 2024

@mrava87 I actually tested the yml file as it current stands, it didnt install the correct numpy. The only way I could get it to work was to version pin:

- numpy>=1.21.0,<2
- scipy>=1.11.0,<1.13
- pytorch>=1.2.0,<2.5
 - pip:
  - devito==4.8.6

I'm reluctant to add this to the "standard" environment-dev.yml since that would force everyone to use old versions just so that pytensor still works. I think instead I will remove pytensor from the yml files, and put a check to not execute the pytensor tests if numpy>=2. What do you think?

I agree. In general I would strive for avoiding forcing upper limits on versions and doing that only as a temporary measure... especially when it comes to numpy, which is arguably the most used python package so every serious library will have to ultimately use numpy>2.

For the requirements.txt files, I am happy with the changes to allow tests/doc to be build in the CI... if we face issues in future we will decide what to do with respect to pytensor, but hopefully the PR you pointed to will be ready soon and the problem fixed 🚀

Feel free to merge after you add the check to the pytensor tests

@cako cako merged commit d832ec6 into PyLops:dev Nov 20, 2024
13 checks passed
@cako cako deleted the feature-pytensor branch November 20, 2024 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants