Skip to content

Chore: refactor InvarFitting #3266

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.

Sign up for GitHub

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

Jump to bottom
Merged
wanghan-iapcm merged 46 commits into from
Feb 16, 2024
Merged

Chore: refactor InvarFitting #3266

wanghan-iapcm merged 46 commits into from
Feb 16, 2024

Conversation

@anyangml
Copy link
Collaborator

@anyangml anyangml commented Feb 13, 2024

No description provided.

@anyangml anyangml marked this pull request as draft February 13, 2024 10:38
@anyangml anyangml self-assigned this Feb 13, 2024
@codecov
Copy link

codecov bot commented Feb 13, 2024 •
edited
Loading

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (43f17da) 74.98% compared to head (e125be1) 74.99%.

Files Patch % Lines
deepmd/pt/model/task/fitting.py 97.14% 4 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##            devel    #3266   +/-   ##
=======================================
  Coverage   74.98%   74.99%           
=======================================
  Files         384      384           
  Lines       33522    33547   +25     
  Branches     1604     1604           
=======================================
+ Hits        25136    25157   +21     
- Misses       7525     7529    +4     
  Partials      861      861

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@anyangml
Copy link
Collaborator Author

anyangml commented Feb 13, 2024

@wanghan-iapcm, this is only a draft, not tested, but it's what i have in mind. feels like we can reuse a large portion of the code from InvarFitting, we only need to change the output dim from 1 to 3or 9, and transform the output with gr. If that's the case, we might want to create a GeneralFitting to encapsulate the common methods, and refactor InvarFitting as a child class.

@wanghan-iapcm
Copy link
Collaborator

wanghan-iapcm commented Feb 13, 2024

please note that the predictions of dipole and polar are rotationally equivariant.

@anyangml
Copy link
Collaborator Author

anyangml commented Feb 14, 2024

please note that the predictions of dipole and polar are rotationally equivariant.

I thought this handles the rotational equivariance,

deepmd-kit/deepmd/pt/model/task/dipole.py

Line 203 in 9bc9900

torch.bmm(outs, gr).squeeze(-2).view(nframes, nloc, 3)

github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 14, 2024
View reviewed changes
@anyangml anyangml mentioned this pull request Feb 15, 2024
Closed
anyangml
anyangml commented Feb 15, 2024
View reviewed changes
@anyangml anyangml added the Test CUDA Trigger test CUDA workflow label Feb 15, 2024
@github-actions github-actions bot removed the Test CUDA Trigger test CUDA workflow label Feb 15, 2024
@anyangml anyangml added the Test CUDA Trigger test CUDA workflow label Feb 15, 2024
@github-actions github-actions bot removed the Test CUDA Trigger test CUDA workflow label Feb 15, 2024
njzjz
njzjz reviewed Feb 15, 2024
View reviewed changes
@anyangml anyangml requested a review from iProzd February 16, 2024 02:12
@anyangml
Copy link
Collaborator Author

anyangml commented Feb 16, 2024 •
edited
Loading

@wanghan-iapcm UT 'test_make_hessian_model` fails on CUDA, not sure if it was tested before.

@anyangml anyangml added the Test CUDA Trigger test CUDA workflow label Feb 16, 2024
@github-actions github-actions bot removed the Test CUDA Trigger test CUDA workflow label Feb 16, 2024
njzjz
njzjz reviewed Feb 16, 2024
View reviewed changes
@anyangml anyangml requested a review from njzjz February 16, 2024 03:24
@anyangml anyangml added the Test CUDA Trigger test CUDA workflow label Feb 16, 2024
@github-actions github-actions bot removed the Test CUDA Trigger test CUDA workflow label Feb 16, 2024
@anyangml anyangml added the Test CUDA Trigger test CUDA workflow label Feb 16, 2024
@github-actions github-actions bot removed the Test CUDA Trigger test CUDA workflow label Feb 16, 2024
njzjz
njzjz approved these changes Feb 16, 2024
View reviewed changes
@wanghan-iapcm wanghan-iapcm merged commit 15f8d25 into deepmodeling:devel Feb 16, 2024
@njzjz njzjz mentioned this pull request Apr 2, 2024
Closed
@njzjz njzjz linked an issue May 1, 2024 that may be closed by this pull request
[Feature Request] dpmodel and pt fitting should export several methods #3273
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@njzjz njzjz njzjz approved these changes

@iProzd iProzd iProzd approved these changes

@wanghan-iapcm wanghan-iapcm Awaiting requested review from wanghan-iapcm

Assignees

@anyangml anyangml

Labels

Python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Feature Request] dpmodel and pt fitting should export several methods

4 participants

@anyangml @wanghan-iapcm @njzjz @iProzd