Remove physicsnemo compatibility layer - #1997
coreyjadams wants to merge 1 commit into
Conversation
CODEOWNERS review mapCurrent for commit ⏳ @megnvidia — 1 file(s)
⏳ @ram-cherukuri — 1 file(s)
No CODEOWNER
Comment |
|
The code removal appears safe to merge, but the migration documentation and discovery fallback should be corrected so users can reliably translate removed import paths. FindingsSummaryThis PR removes the opt-in pre-v2.0 import compatibility layer, its activation environment variable, import-linter configuration, and dedicated tests. It also updates release and migration documentation plus the discovery skill’s renamed-API fallback.
Reviews (1) · Last reviewed commit: "Remove physicsnemo compatibility layer f..." |
| should suffice. | ||
| PhysicsNeMo v2.0 through v2.2 shipped an opt-in `physicsnemo.compat` module that | ||
| aliased pre-v2.0 import paths onto their new locations. That layer was removed in | ||
| v2.3. Update import paths using the tables in this guide; the `PHYSICSNEMO_ENABLE_COMPAT` |
There was a problem hiding this comment.
This section directs users to “the tables in this guide,” but the guide does not include a table for the aliases being removed. For example, it does not document that physicsnemo.models.diffusion.Conv2D moved to physicsnemo.nn.Conv2d, physicsnemo.models.diffusion.SongUNet moved to physicsnemo.models.diffusion_unets.SongUNet, or physicsnemo.models.diffusion.EDMPrecond moved to physicsnemo.diffusion.preconditioners.EDMPrecond. Once the compatibility map is deleted, users encountering these broken imports will not have a reliable migration reference. Please preserve the removed module and object mappings as a table in this guide.
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!
| # 7c. If not found where expected — check compat layer for renames | ||
| Read physicsnemo/compat/__init__.py | ||
| # 7c. If not found where expected — check the rename tables for moved paths | ||
| Grep -n "<ClassName>" v2.0-MIGRATION-GUIDE.md CHANGELOG.md |
There was a problem hiding this comment.
Function Fallback Uses Class Placeholder
The fallback command hardcodes <ClassName>, even though the preceding workflow also searches for functions using <func_name>. When a renamed function is not found, following this recipe literally searches for the wrong placeholder and may cause the discovery skill to omit a valid API. Use a neutral placeholder such as <name> for both cases.
| Grep -n "<ClassName>" v2.0-MIGRATION-GUIDE.md CHANGELOG.md | |
| Grep -n "<name>" v2.0-MIGRATION-GUIDE.md CHANGELOG.md |
PhysicsNeMo Pull Request
We are now in version v2.3, it is safe to remove the v2.0 compatibility shims.
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.