Conversation
Greptile SummaryThis PR upstreams
Important Files Changed
Reviews (2): Last reviewed commit: "address all greptile comments" | Re-trigger Greptile |
…derivative functionals
003d65d to
af00635
Compare
|
/blossom-ci |
|
Updated the branch after pulling in the changes from the latest main (after #1545 was merged). Ready for review |
coreyjadams
left a comment
There was a problem hiding this comment.
I have mostly left comments about the overall structure, but a few things can be cleaned up too
|
I approved for the code owner edit, but man I dont understand why this is getting added tbh. |
Think of it as an abstraction on top of Functionals. Why do you think it is worse? Any design changes needed? |
|
I think its the fact that psn.sym was a framework centered around all these symbolic representations. So the symbolic description of PDEs worked well there and had create synergy with the entirety of the package (since it was build around symbolic PDEs) While here symbolic representations are now just kinda this stripped down version which has a lot less synergy with the rest of the package. I not saying any of the implementation here is better or worse. I'm talking higher level, but this is not my judgement call to make... Seeing the graph representations getting added seemed out of place. I do like the general idea of building on top of the functionals |
Thanks for the context @NickGeneva ! Discussed this offline but will also share here for completeness. Yes, the motivation of this PR is hence to only bring in the base |
|
@ktangsali what do you think of @peterdsharpe 's suggestion to use mesh for the adjacency computation instead of numba? I really like keeping the pyproject.toml free of unneeded deps. If we really, truly need numba, ok, let's do it. If we can do it without it, and it's not expected to be a critical path feature, can we avoid it? |
I have updated the implementation to use Mesh now. Removed numba imports :) |
|
/blossom-ci |
coreyjadams
left a comment
There was a problem hiding this comment.
Thanks @ktangsali for taking the time to work on the numba dependency!
PhysicsNeMo Pull Request
Description
First PR of Sym upstream efforts. Examples would be ported in a separate PR.
This PR depends on #1545
Summary
Adds
physicsnemo.sym— a symbolic PDE residual computation module that enablesphysics-informed training by automatically computing spatial derivatives and
evaluating PDE residuals. This is the upstreamed and modernized version of the
PhysicsInformer from physicsnemo-sym, rewired to use the PhysicsNeMo derivative
functionals (
nn.functional.derivatives) as its computational backend.What it does
PhysicsInformer takes a user-defined PDE (expressed symbolically via SymPy) and a
gradient method, then automatically:
Users define their PDEs inline using standard SymPy:
Key design decisions
Tests
Ported the exact same test methodology from physicsnemo-sym:
test_gradients.py — validates all 5 gradient methods against analytical derivatives of u = sin(x)*sin(8y)*sin(4z), threshold < 0.2
test_phy_informer.py — validates end-to-end Navier-Stokes residuals (continuity + momentum_x) across all 5 methods, threshold < 0.5
Checklist
Dependencies
Review Process
All PRs are reviewed by the PhysicsNeMo team before merging.
Depending on which files are changed, GitHub may automatically assign a maintainer for review.
We are also testing AI-based code review tools (e.g., Greptile), which may add automated comments with a confidence score.
This score reflects the AI’s assessment of merge readiness and is not a qualitative judgment of your work, nor is
it an indication that the PR will be accepted / rejected.
AI-generated feedback should be reviewed critically for usefulness.
You are not required to respond to every AI comment, but they are intended to help both authors and reviewers.
Please react to Greptile comments with 👍 or 👎 to provide feedback on their accuracy.