add BaseModel; store type in serialization #3335

njzjz · 2024-02-25T02:53:06Z

No description provided.

Signed-off-by: Jinzhe Zeng <[email protected]>

deepmd/pt/model/model/dp_model.py

+        from deepmd.pt.model.model.dipole_model import (
+            DipoleModel,
+        )


deepmd/pt/model/model/dp_model.py

+        from deepmd.pt.model.model.ener_model import (
+            EnergyModel,
+        )


deepmd/pt/model/model/dp_model.py

+        from deepmd.pt.model.model.polar_model import (
+            PolarModel,
+        )


deepmd/pt/model/model/dp_model.py

deepmd/dpmodel/model/dp_model.py

deepmd/pt/model/model/dp_zbl_model.py

@@ -17,7 +20,8 @@
 DPZBLModel_ = make_model(DPZBLLinearAtomicModel)


-class DPZBLModel(DPZBLModel_):
+@BaseModel.register("zbl")
+class DPZBLModel(DPZBLModel_, BaseModel):


codecov · 2024-02-25T02:59:12Z

Codecov Report

Attention: Patch coverage is 87.50000% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 75.57%. Comparing base (261c802) to head (d3ef0a4).
Report is 2 commits behind head on devel.

Files	Patch %	Lines
deepmd/dpmodel/model/base_model.py	85.10%	7 Missing ⚠️
deepmd/utils/plugin.py	76.47%	4 Missing ⚠️
deepmd/pt/model/model/dp_model.py	88.88%	2 Missing ⚠️
...pmd/dpmodel/atomic_model/make_base_atomic_model.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            devel    #3335      +/-   ##
==========================================
+ Coverage   75.52%   75.57%   +0.05%     
==========================================
  Files         408      409       +1     
  Lines       34277    34377     +100     
  Branches     1604     1600       -4     
==========================================
+ Hits        25888    25982      +94     
- Misses       7528     7534       +6     
  Partials      861      861

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

Signed-off-by: Jinzhe Zeng <[email protected]>

deepmd/pt/model/model/dp_model.py

-DPModel = make_model(DPAtomicModel)
+
+@BaseModel.register("standard")
+class DPModel(make_model(DPAtomicModel), BaseModel):


wanghan-iapcm

I do not understand why the serialization and deserialization cannot be put int BaseBaseModel.
would the following inheritance work?

#       torch.nn.Module        BaseAtomicModel        BaseBaseModel
#             |                       |                    |
#             -------------------------                    |
#                         |                                |
#                    DPAtomicModel                         |
#                         |                                |
#                make_model(DPAtomicModel)                 |
#                         |                                |
#                         ----------------------------------
#                                           |
#                                         DPModel

in our concept a model is an atomic model, therefore, the base model seems to be a facilitate that provides the plugin, other methods like the get_rcut, get_dim_fparam... seems to be inherited from atomic model rather than model .

deepmd/dpmodel/model/base_model.py

deepmd/pt/model/model/model.py

Co-authored-by: Han Wang <[email protected]> Signed-off-by: Jinzhe Zeng <[email protected]>

Signed-off-by: Jinzhe Zeng <[email protected]>

This reverts commit 054fba6.

Signed-off-by: Jinzhe Zeng <[email protected]>

njzjz · 2024-02-25T22:47:46Z

I do not understand why the serialization and deserialization cannot be put int BaseBaseModel.

Done.

would the following inheritance work?

#       torch.nn.Module        BaseAtomicModel        BaseBaseModel
#             |                       |                    |
#             -------------------------                    |
#                         |                                |
#                    DPAtomicModel                         |
#                         |                                |
#                make_model(DPAtomicModel)                 |
#                         |                                |
#                         ----------------------------------
#                                           |
#                                         DPModel

We cannot store the plugins of dpmodel and pt in the same class. There must be seperately classes to initialize the plugin separately.

in our concept a model is an atomic model, therefore, the base model seems to be a facilitate that provides the plugin, other methods like the get_rcut, get_dim_fparam... seems to be inherited from atomic model rather than model .

It looks to me SpinModel in #3301 is not inherited from an atomic model.

deepmd/pt/model/model/model.py

Signed-off-by: Jinzhe Zeng <[email protected]>

wanghan-iapcm · 2024-02-26T14:07:21Z

We cannot store the plugins of dpmodel and pt in the same class. There must be seperately classes to initialize the plugin separately.

it should be possible to make_base_model for different backends, see #3340

Signed-off-by: Jinzhe Zeng <[email protected]>

deepmd/dpmodel/model/base_model.py

deepmd/utils/plugin.py

deepmd/dpmodel/model/base_model.py

Signed-off-by: Jinzhe Zeng <[email protected]>

njzjz added 2 commits February 24, 2024 21:40

basemodel; store type

b374abc

Signed-off-by: Jinzhe Zeng <[email protected]>

use DeepEval

325ff74

Signed-off-by: Jinzhe Zeng <[email protected]>

njzjz requested review from iProzd and wanghan-iapcm February 25, 2024 02:53

github-actions bot added the Python label Feb 25, 2024

github-advanced-security bot found potential problems Feb 25, 2024

View reviewed changes

njzjz added 2 commits February 24, 2024 22:18

fix DPModel.__new__

99dbc7b

Signed-off-by: Jinzhe Zeng <[email protected]>

fallback to DPModel

7fbaf71

Signed-off-by: Jinzhe Zeng <[email protected]>

github-advanced-security bot found potential problems Feb 25, 2024

View reviewed changes

njzjz mentioned this pull request Feb 25, 2024

serialize should contain type information #3319

Closed

3 tasks

wanghan-iapcm reviewed Feb 25, 2024

View reviewed changes

deepmd/dpmodel/model/base_model.py Outdated Show resolved Hide resolved

deepmd/pt/model/model/model.py Outdated Show resolved Hide resolved

deepmd/pt/model/model/model.py Outdated Show resolved Hide resolved

deepmd/pt/model/model/model.py Outdated Show resolved Hide resolved

njzjz and others added 2 commits February 25, 2024 16:58

Update deepmd/dpmodel/model/base_model.py

c2fb0be

Co-authored-by: Han Wang <[email protected]> Signed-off-by: Jinzhe Zeng <[email protected]>

Apply suggestions from code review

7af6ce3

Co-authored-by: Han Wang <[email protected]> Signed-off-by: Jinzhe Zeng <[email protected]>

njzjz marked this pull request as draft February 25, 2024 22:00

njzjz added 7 commits February 25, 2024 17:23

Merge branch 'devel' into basemodel

fd455b7

fix indent

1fe4def

Signed-off-by: Jinzhe Zeng <[email protected]>

combine deserialize

74c4116

Signed-off-by: Jinzhe Zeng <[email protected]>

fix typo

ffc7dfc

Signed-off-by: Jinzhe Zeng <[email protected]>

use self as type hint

054fba6

Signed-off-by: Jinzhe Zeng <[email protected]>

Revert "use self as type hint"

2f9d56f

This reverts commit 054fba6.

add serialize

4a17c1f

Signed-off-by: Jinzhe Zeng <[email protected]>

njzjz marked this pull request as ready for review February 25, 2024 22:48

njzjz requested a review from wanghan-iapcm February 25, 2024 22:48

njzjz mentioned this pull request Feb 25, 2024

Support DPSpin for AtomicModel #3301

Merged

11 tasks

iProzd reviewed Feb 26, 2024

View reviewed changes

deepmd/pt/model/model/model.py Outdated Show resolved Hide resolved

deepmd/pt/model/model/model.py Outdated Show resolved Hide resolved

Apply suggestions from code review

9681276

Signed-off-by: Jinzhe Zeng <[email protected]>

add make_base_model

67b83ed

Signed-off-by: Jinzhe Zeng <[email protected]>

fix error message

fbd1300

Signed-off-by: Jinzhe Zeng <[email protected]>

njzjz commented Feb 26, 2024

View reviewed changes

deepmd/dpmodel/model/base_model.py Show resolved Hide resolved

deepmd/utils/plugin.py Show resolved Hide resolved

njzjz requested a review from iProzd February 26, 2024 22:43

wanghan-iapcm reviewed Feb 27, 2024

View reviewed changes

deepmd/dpmodel/model/base_model.py Outdated Show resolved Hide resolved

move get_model_def_script to make_base_atomic_model

d3ef0a4

Signed-off-by: Jinzhe Zeng <[email protected]>

njzjz requested a review from wanghan-iapcm February 27, 2024 01:11

wanghan-iapcm approved these changes Feb 27, 2024

View reviewed changes

wanghan-iapcm added this pull request to the merge queue Feb 27, 2024

Merged via the queue into deepmodeling:devel with commit 854d998 Feb 27, 2024
48 checks passed

njzjz linked an issue Mar 1, 2024 that may be closed by this pull request

serialize should contain type information #3319

Closed

3 tasks

njzjz mentioned this pull request Apr 2, 2024

[TYPO] #3635

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add BaseModel; store type in serialization #3335

add BaseModel; store type in serialization #3335

njzjz commented Feb 25, 2024

codecov bot commented Feb 25, 2024 •

edited

Loading

wanghan-iapcm left a comment

njzjz commented Feb 25, 2024 •

edited

Loading

wanghan-iapcm commented Feb 26, 2024

add BaseModel; store type in serialization #3335

add BaseModel; store type in serialization #3335

Conversation

njzjz commented Feb 25, 2024

codecov bot commented Feb 25, 2024 • edited Loading

Codecov Report

wanghan-iapcm left a comment

Choose a reason for hiding this comment

njzjz commented Feb 25, 2024 • edited Loading

wanghan-iapcm commented Feb 26, 2024

codecov bot commented Feb 25, 2024 •

edited

Loading

njzjz commented Feb 25, 2024 •

edited

Loading