Repository navigation
Conversation
…s_informed example Closes NVIDIA#2020. Signed-off-by: aaguadob <aimar.a1000@gmail.com>
CODEOWNERS review mapCurrent for commit No CODEOWNER
Comment |
|
[Medium risk] Fixes time derivative handling in a physics simulation example. The PR appears safe to merge, though the corrected time-dependent path should have a regression test. Findings
SummaryThe PR makes the example’s diffusion field depend on
Reviews (1) · Last reviewed commit: "Fix Diffusion(time=True) dropping the ti..." |
| if time: | ||
| iv["t"] = Symbol("t") |
There was a problem hiding this comment.
Time-dependent path lacks coverage
Please add a test that checks Diffusion(time=True) retains a nonzero time derivative. Both existing example callers set time=False, so the behavior fixed here is not exercised and could silently regress.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Added in 66c888c: examples/cfd/darcy_physics_informed/tests/test_utils.py checks that Diffusion(time=True) keeps dT/dt (with T a function of t) and that time=False gives -div(k grad u) - Q.
Signed-off-by: aaguadob <aimar.a1000@gmail.com>
…s_informed example
Closes #2020.
PhysicsNeMo Pull Request
Description
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.