Skip to content

Reconstruct the interface of model compression#921

Merged
amcadmus merged 12 commits intodeepmodeling:develfrom
denghuilu:reconstruct-model-compression
Aug 9, 2021
Merged

Reconstruct the interface of model compression#921
amcadmus merged 12 commits intodeepmodeling:develfrom
denghuilu:reconstruct-model-compression

Conversation

@denghuilu
Copy link
Member

@denghuilu denghuilu commented Aug 4, 2021

Before this pull request, the model compression module within the deepmd-kit would require the users to provide the training script as well as the training data to compress a given frozen model. Herein, this pull request brings a more useful and robust interface of model compression, which removes the dependences on training script and data from the model compression.

The main changes are:

The next pull request will add an item within the convert-from interface to allow the users to upgrade their existing frozen models to adapt the new interface of model compression.

@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2021

Codecov Report

Merging #921 (a1f087d) into devel (ee0ed99) will increase coverage by 0.11%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##            devel     #921      +/-   ##
==========================================
+ Coverage   75.42%   75.53%   +0.11%     
==========================================
  Files          85       85              
  Lines        6730     6770      +40     
==========================================
+ Hits         5076     5114      +38     
- Misses       1654     1656       +2     
Impacted Files Coverage Δ
deepmd/entrypoints/freeze.py 74.46% <ø> (ø)
deepmd/entrypoints/main.py 88.29% <ø> (-0.13%) ⬇️
deepmd/entrypoints/compress.py 91.11% <83.33%> (-1.39%) ⬇️
deepmd/common.py 83.44% <86.66%> (+0.35%) ⬆️
deepmd/train/trainer.py 71.92% <96.77%> (+0.85%) ⬆️
deepmd/entrypoints/train.py 87.97% <100.00%> (+1.21%) ⬆️
deepmd/utils/argcheck.py 87.58% <100.00%> (+0.04%) ⬆️
deepmd/utils/errors.py 100.00% <100.00%> (ø)
deepmd/utils/data.py 90.33% <0.00%> (-0.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee0ed99...a1f087d. Read the comment docs.

@njzjz
Copy link
Member

njzjz commented Aug 5, 2021

I have another question. What is the behavior if one uses another JSON to restart the training? For example, in active learning cycles, we can continue training from the last iteration.

@denghuilu
Copy link
Member Author

I have another question. What is the behavior if one uses another JSON to restart the training? For example, in active learning cycles, we can continue training from the last iteration.

The model will save the latest json file.

@amcadmus
Copy link
Member

amcadmus commented Aug 5, 2021

I have another question. What is the behavior if one uses another JSON to restart the training? For example, in active learning cycles, we can continue training from the last iteration.

In the init-model mode, overwriting the existing training script should be the expected behavior.

In the restart mode, the training script are expected to be the same.

@denghuilu denghuilu requested review from amcadmus and njzjz August 6, 2021 09:22
Copy link
Member

@njzjz njzjz left a comment

Choose a reason for hiding this comment

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

Also, please fix lint warnings.

@njzjz
Copy link
Member

njzjz commented Aug 7, 2021

if self.is_compress == False:
# Usually, the type number of the model should be equal to that of the data
# However, nt_model > nt_data should be allowed, since users may only want to
# train using a dataset that only have some of elements
assert (self.ntypes >= data.get_ntypes()), "ntypes should match that found in data"
self.stop_batch = stop_batch
# self.batch_size = data.get_batch_size()
if self.numb_fparam > 0 :
log.info("training with %d frame parameter(s)" % self.numb_fparam)
else:
log.info("training without frame parameter")
# self.type_map = data.get_type_map()
if self.is_compress == False:
# Usually, the type number of the model should be equal to that of the data
# However, nt_model > nt_data should be allowed, since users may only want to
# train using a dataset that only have some of elements
assert (self.ntypes >= data.get_ntypes()), "ntypes should match that found in data"

Duplicated lines

@amcadmus amcadmus requested a review from njzjz August 9, 2021 02:14
@amcadmus amcadmus merged commit 4a9943e into deepmodeling:devel Aug 9, 2021
@amcadmus amcadmus mentioned this pull request Aug 12, 2021
@denghuilu denghuilu deleted the reconstruct-model-compression branch August 21, 2021 09:46
gzq942560379 pushed a commit to HPC-AI-Team/deepmd-kit that referenced this pull request Sep 2, 2021
* remove dependences on training script and data from model compression

* reset function update_one_sel in train.py

* update the doc of model compression

* fix bug in UT

* optimize code for reviewer's comments

* undo changes to constant variables

* Update common.py

* update code structure of DPTrainer

* fix lint warnings in common.py

* fix duplicated lines within trainer.py

* Update trainer.py

* rm default values with False optional in argcheck.py
njzjz added a commit to njzjz/deepmd-kit that referenced this pull request Aug 21, 2022
deepmodeling#921 discussed that the tensors are compressed in the graph file. But it looks no... So at least we remove white space.
wanghan-iapcm pushed a commit that referenced this pull request Aug 24, 2022
#921 discussed that the tensors are compressed in the graph file. But it looks no... So at least we remove white space.
mingzhong15 pushed a commit to mingzhong15/deepmd-kit that referenced this pull request Jan 15, 2023
deepmodeling#921 discussed that the tensors are compressed in the graph file. But it looks no... So at least we remove white space.
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.

[Feature Request] Save the range of embedding matrix input in the DP model [Feature Request] Save the input script to DP model

6 participants