Gradients updates, Incompressible Navier Stokes PDE

[ktangsali]

PhysicsNeMo Pull Request

Description

Updates to Least Squares gradients

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.
• The CHANGELOG.md is up to date with these changes.
• An issue is linked to this pull request.

Dependencies

[ktangsali]

/blossom-ci

[peterdsharpe]

Looks good! The core of this PR (the PDE part) looks good.

There are some code quality changes to fix before merging, with repeated themes highlighted here:

• Builtin Type Hinting: Type hints should use builtin types since Python 3.10: List should be list; Dict should be dict, and so on.
• Use "Sequence" More: I notice that we use a lot of type hints on input arguments that are list. I suspect that for almost-all of these, we don't actually mean list - we mean Sequence, which can be imported from typing. This sounds like a minor difference, but it becomes an issue for end users when they give these arguments as, say tuples. All of a sudden, when they use an IDE with a modern static-type-checking language server (like ty via VSCode/Cursor, or PyCharm's backend), they get type warning errors all over the place - this makes it hard for end users to distinguish what's an actual type mistake, and what is simply an overly-restrictive type hint.
• String-Formatting: We should use f-strings where possible (i.e., almost-always), for readability.
• Comprehensions: list and dictionary comprehensions are not just more readable, but also more performant that direct loop+indexing in recent Python versions. We should use them where possible.

None of these individually are show-stoppers, but collectively I think there are enough of them to warrant fixing.

[ktangsali]

Looks good! The core of this PR (the PDE part) looks good.

There are some code quality changes to fix before merging, with repeated themes highlighted here:

• Builtin Type Hinting: Type hints should use builtin types since Python 3.10: List should be list; Dict should be dict, and so on.
• Use "Sequence" More: I notice that we use a lot of type hints on input arguments that are list. I suspect that for almost-all of these, we don't actually mean list - we mean Sequence, which can be imported from typing. This sounds like a minor difference, but it becomes an issue for end users when they give these arguments as, say tuples. All of a sudden, when they use an IDE with a modern static-type-checking language server (like ty via VSCode/Cursor, or PyCharm's backend), they get type warning errors all over the place - this makes it hard for end users to distinguish what's an actual type mistake, and what is simply an overly-restrictive type hint.
• String-Formatting: We should use f-strings where possible (i.e., almost-always), for readability.
• Comprehensions: list and dictionary comprehensions are not just more readable, but also more performant that direct loop+indexing in recent Python versions. We should use them where possible.

None of these individually are show-stoppers, but collectively I think there are enough of them to warrant fixing.

Thanks Peter for the review! I have addressed your feedback. Please have a look once more.

[peterdsharpe]

Re-review complete! A few new items to fix; biggest one being a mutable default argument in a function.

[peterdsharpe]

Looking great! One last comment to fix, with another mutable default argument. Pre-emptively approving so that I'm not the bottleneck

[ktangsali]

/blossom-ci