Skip to content

Conversation

@coreyjadams
Copy link
Collaborator

PhysicsNeMo Pull Request

Description

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

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 Dec 11, 2025

Greptile Overview

Greptile Summary

This PR adds CI infrastructure to test package installation via both pip and uv across Python 3.11-3.13 and drops support for Python 3.10. The changes also improve MacOS compatibility by conditionally enabling pin_memory based on CUDA availability.

Key Changes:

  • Created new install-ci.yml workflow testing pip and uv installations on Ubuntu and MacOS
  • Updated minimum Python version from 3.10 to 3.11
  • Fixed MacOS compatibility by making pin_memory conditional on CUDA availability
  • Updated CUDA dependencies from generic to cu13-specific packages
  • Removed MacOS-specific test skips that are no longer needed
  • Added CUDA availability check to test_means_var

Issues Found:

  • CI workflow requires pip >= 25.1 for --group flag but doesn't enforce the version upgrade
  • README badge points to author's fork (coreyjadams/physicsnemo) instead of main repository (nvidia/physicsnemo)
  • UV_PREVIEW environment variable is deprecated and unnecessary

Important Files Changed

File Analysis

Filename Score Overview
.github/workflows/install-ci.yml 3/5 Added CI workflow testing pip and uv installations across Python 3.11-3.13, but pip version requirement not enforced and UV_PREVIEW flag is deprecated
README.md 3/5 Added Install CI badge, but badge URL incorrectly points to author's fork instead of nvidia/physicsnemo repository
physicsnemo/nn/neighbors/_radius_search/_warp_impl.py 5/5 Fixed pin_memory flag to check CUDA availability, enabling MacOS compatibility without breaking CUDA functionality

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.

Additional Comments (3)

  1. .github/workflows/install-ci.yml, line 33 (link)

    logic: --group flag requires pip >= 25.1, but older versions may be installed by default on runners

  2. README.md, line 10 (link)

    logic: Badge URL points to coreyjadams/physicsnemo (author's fork), should point to nvidia/physicsnemo

  3. .github/workflows/install-ci.yml, line 63 (link)

    style: UV_PREVIEW is deprecated - uv sync natively supports --group without preview mode

6 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

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