Skip to content

Jax ml operator fix #4041

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

Merged
merged 31 commits into from
Feb 25, 2025
Merged

Jax ml operator fix #4041

merged 31 commits into from
Feb 25, 2025

Conversation

Ig-dolci
Copy link
Contributor

@Ig-dolci Ig-dolci commented Feb 14, 2025

Description

Depends on firedrakeproject/ufl#60

  • Fixed an issue in firedrake.ml.jax.ml_operator where argument_slots was incorrectly specified. I wrote it as optional:

    argument_slots: Optional[tuple[Union[ufl.coefficient.BaseCoefficient, ufl.argument.BaseArgument]]] = ()
  • Clarified that if argument_slots is not provided, ML_Operator will automatically write it.

  • Test the results involving the Neural operators are in right function space.

  • Test jax and pytorch operators in Firedrake CI.

Copy link

github-actions bot commented Feb 14, 2025

TestsPassed ✅Skipped ⏭️Failed ❌
Firedrake complex8269 ran6595 passed1674 skipped0 failed

Copy link

github-actions bot commented Feb 14, 2025

TestsPassed ✅Skipped ⏭️Failed ❌
Firedrake real8198 ran7500 passed698 skipped0 failed

@Ig-dolci Ig-dolci changed the title Does this should be optional? Jax ml operator fix Feb 14, 2025
@Ig-dolci Ig-dolci marked this pull request as ready for review February 17, 2025 14:11
@Ig-dolci
Copy link
Contributor Author

@pbrubeck @dham Firedrake pip installs PR depends on the approval of UFL PR 348 and this PR.

@Ig-dolci
Copy link
Contributor Author

Ig-dolci commented Feb 21, 2025

The failure in the docs building is because of a recent Sphinx update.

connorjward
connorjward previously approved these changes Feb 25, 2025
Copy link
Contributor

@connorjward connorjward left a comment

Choose a reason for hiding this comment

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

@pbrubeck if you are also happy it would be fantastic if this could be merged ASAP as it is holding up the pip PR. Naturally we need to update our UFL fork at the same time.

@pbrubeck pbrubeck enabled auto-merge (squash) February 25, 2025 13:13
@Ig-dolci Ig-dolci disabled auto-merge February 25, 2025 13:58
@Ig-dolci Ig-dolci enabled auto-merge (squash) February 25, 2025 14:00
@connorjward connorjward disabled auto-merge February 25, 2025 15:14
@connorjward connorjward merged commit a84c071 into master Feb 25, 2025
19 of 20 checks passed
@connorjward connorjward deleted the dolci/jax_ml_operator branch February 25, 2025 15:15
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.

4 participants