Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade torch to 2.4.0 in pyproject.toml #382

Closed
wants to merge 54 commits into from

Conversation

kenko911
Copy link
Contributor

@kenko911 kenko911 commented Oct 6, 2024

Summary

Upgrade torch to 2.4.0 in pyproject.toml to see if the united test for numpy 2.0.1 can be fixed

Checklist

  • Google format doc strings added. Check with ruff.
  • Type annotations included. Check with mypy.
  • Tests added for new features/fixes.
  • If applicable, new classes/functions/modules have duecredit @due.dcite decorators to reference relevant papers by DOI (example)

Tip: Install pre-commit hooks to auto-check types and linting before every commit:

pip install -U pre-commit
pre-commit install

kenko911 and others added 30 commits June 22, 2024 09:24
Signed-off-by: Tsz Wai Ko <[email protected]>
@kenko911 kenko911 requested a review from shyuep as a code owner October 6, 2024 01:05
Copy link
Contributor

coderabbitai bot commented Oct 6, 2024

Walkthrough

The changes in this pull request primarily involve updates to the pyproject.toml and requirements.txt files for the "matgl" project, specifically modifying dependency version constraints for the dgl and torch libraries. Additionally, the test cases for the BondExpansion class in tests/layers/test_bond.py have been adjusted to reflect a change in expected output dimensions for one of the tests. Modifications to the MLP_norm class in src/matgl/layers/_core.py enhance its configurability by introducing a new parameter for bias handling and refining the handling of normalization layers.

Changes

File Change Summary
pyproject.toml, requirements.txt Updated dgl version constraint from <=2.4.0 to <=2.2.1 in both files. Updated torch version constraint from <=2.2.1 to <=2.4.0 in pyproject.toml.
tests/layers/test_bond.py Modified test_exp_normal to change the expected shape of bond_basis from (28, 9) to (27, 9). Added assertion for second graph g2 with shape (2, 9). Other tests remain unchanged.
src/matgl/layers/_core.py Updated MLP_norm constructor to include use_bias parameter and explicitly cast norm_kwargs to dict. Adjusted logic for normalization layers based on the normalization parameter.

Possibly related PRs

  • Matgl depends on torch==2.2.1 #273: This PR updates the requirements.txt file to specify that the matgl package now depends on torch==2.2.1, which is relevant as the main PR updates the version constraint for torch to allow up to 2.4.0.
  • Bump dgl from 2.1.0 to 2.2.1 #302: This PR updates the requirements.txt and pyproject.toml files to change the version of dgl from 2.1.0 to 2.2.1, which is related since the main PR also modifies the version constraint for dgl.
  • Bump torch from 2.2.1 to 2.4.0 #308: This PR proposes to update the torch library from 2.2.1 to 2.4.0, which directly relates to the main PR's change in the version constraint for torch.
  • Update DGL version for torch v2.4.1 #351: This PR updates the version constraints for dgl in both requirements.txt and pyproject.toml, changing it from dgl<=2.2.1 to dgl<=2.4.0, which is directly relevant to the changes made in the main PR.

Suggested labels

dependencies, python

Suggested reviewers

  • shyuep

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
src/matgl/layers/_core.py (1)

Line range hint 85-89: Typographical error in the docstring

In the docstring for the __init__ method of MLP_norm, the parameter description for activation is duplicated. Please remove the redundant word.

Apply this diff to fix the typo:

             Args:
                 dims: Dimensions of each layer of MLP.
-                activation: activation: Activation function.
+                activation: Activation function.
                 activate_last: Whether to apply activation to last layer.
                 use_bias: Whether to use bias.
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 8b63fdc and 7209f48.

📒 Files selected for processing (1)
  • src/matgl/layers/_core.py (2 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/matgl/layers/_core.py (2)

5-5: Importing 'cast' is appropriate for type safety

The inclusion of cast from the typing module is necessary for the explicit type casting used later in the code.


133-133: Ensuring type safety with explicit cast

Using cast(dict, norm_kwargs) ensures that norm_kwargs is recognized as a dictionary by static type checkers. This is appropriate and helps maintain type safety.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
pyproject.toml (1)

53-56: Summary of dependency changes and suggested next steps

This PR makes two key changes to dependencies:

  1. Downgrades the maximum allowed version of dgl from 2.4.0 to 2.2.1
  2. Upgrades the maximum allowed version of torch from 2.2.1 to 2.4.0

While the torch upgrade aligns with the PR objectives, the dgl downgrade is unexpected. To ensure these changes are appropriate and don't introduce issues:

  1. Clarify the rationale behind the dgl version constraint change.
  2. Verify compatibility between the new torch version and the constrained dgl version.
  3. Run the full test suite to catch any regressions or compatibility issues.
  4. Update the PR description to explain both changes, not just the torch upgrade.
  5. Consider updating any documentation that might reference these dependency versions.
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 7209f48 and 10932c0.

📒 Files selected for processing (2)
  • pyproject.toml (1 hunks)
  • requirements.txt (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • requirements.txt
🧰 Additional context used
🔇 Additional comments (2)
pyproject.toml (2)

56-56: LGTM! Verify compatibility with existing codebase

The upgrade of the torch dependency to version <=2.4.0 aligns with the PR objectives. This change could potentially introduce new features or performance improvements.

To ensure a smooth transition:

  1. Verify that all existing torch-dependent code in the project is compatible with torch 2.4.0.
  2. Update any torch-specific code that might leverage new features or changed APIs in version 2.4.0.
  3. Run the test suite to catch any potential regressions.

To help identify potential areas that might need attention, you can run:

#!/bin/bash
# Search for torch imports and usages in the codebase
rg --type python "import torch|from torch" -C 5

# Look for any torch version-specific code or comments
rg --type python "torch.*2\.[0-9]" -C 5

53-53: Verify the intention behind downgrading dgl version constraint

The dgl dependency version constraint has been tightened from <=2.4.0 to <=2.2.1. This change restricts the project to use an older version of dgl, which seems to contradict the PR's objective of upgrading dependencies.

Could you please clarify:

  1. The reason for downgrading the dgl version constraint?
  2. Whether this change is related to addressing the issues with the united test for numpy version 2.0.1 mentioned in the PR description?
  3. If this change has been tested for compatibility with other dependencies, especially the upgraded torch version?

To check for potential compatibility issues, you can run:

@kenko911 kenko911 closed this Oct 14, 2024
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.

2 participants