Skip to content

New tests for patching utilities in patching.py#1456

Open
CharlelieLrt wants to merge 6 commits intoNVIDIA:mainfrom
CharlelieLrt:new-tests-patching-diffusion
Open

New tests for patching utilities in patching.py#1456
CharlelieLrt wants to merge 6 commits intoNVIDIA:mainfrom
CharlelieLrt:new-tests-patching-diffusion

Conversation

@CharlelieLrt
Copy link
Collaborator

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.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 26, 2026

Greptile Summary

Added comprehensive test coverage for patching utilities (BasePatching2D, RandomPatching2D, and GridPatching2D) with 774 lines of well-structured tests including non-regression tests, gradient flow verification, and torch.compile compatibility checks.

  • New test_patching.py provides thorough coverage with parameterized tests for different configurations
  • Created shared test infrastructure in test/diffusion/conftest.py with fixtures for deterministic settings and tolerances
  • Added make_input() helper in helpers.py for consistent test tensor generation
  • Refactored test_preconditioners.py to use shared fixtures, eliminating code duplication
  • Included 21 reference data files (.pth) for non-regression testing

The test structure is excellent with proper use of pytest fixtures, good organization, and comprehensive coverage of edge cases. The PR follows good testing practices including proper random seed management and device-agnostic testing (CPU/CUDA).

Important Files Changed

Filename Overview
test/diffusion/test_patching.py Added comprehensive test suite (774 lines) covering BasePatching2D, RandomPatching2D, and GridPatching2D classes with non-regression tests, gradient flow tests, and torch.compile compatibility tests
test/diffusion/conftest.py New shared fixtures file for diffusion tests, providing deterministic_settings and tolerances fixtures, plus shared constants GLOBAL_SEED, CPU_TOLERANCES, and GPU_TOLERANCES
test/diffusion/helpers.py Added make_input() helper function for creating deterministic test tensors using a separate Generator; updated module docstring to be more generic
test/diffusion/test_preconditioners.py Refactored to remove duplicate fixtures and constants that are now shared in test/diffusion/conftest.py (removed 44 lines including deterministic_settings, tolerances, and seed constants)

Last reviewed commit: 877deee

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

24 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@CharlelieLrt
Copy link
Collaborator Author

/blossom-ci

@CharlelieLrt
Copy link
Collaborator Author

/blossom-ci

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.

1 participant