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

provide an option to skip neighbor stat #1313

Merged
merged 2 commits into from
Nov 29, 2021

Conversation

njzjz
Copy link
Member

@njzjz njzjz commented Nov 26, 2021

Resolves #1088.

Also fixes a bug in update_sel.

Resolves deepmodeling#1088.

Also fixes a bug in `update_sel`.
@njzjz njzjz linked an issue Nov 26, 2021 that may be closed by this pull request
@codecov-commenter
Copy link

Codecov Report

Merging #1313 (2111e46) into devel (843a3c5) will decrease coverage by 11.33%.
The diff coverage is n/a.

❗ Current head 2111e46 differs from pull request most recent head c8ded77. Consider uploading reports for the commit c8ded77 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##            devel    #1313       +/-   ##
===========================================
- Coverage   75.62%   64.28%   -11.34%     
===========================================
  Files          91        5       -86     
  Lines        7494       14     -7480     
===========================================
- Hits         5667        9     -5658     
+ Misses       1827        5     -1822     
Impacted Files Coverage Δ
deepmd/entrypoints/main.py
deepmd/entrypoints/train.py
deepmd/infer/deep_tensor.py
deepmd/entrypoints/__init__.py
deepmd/cluster/__init__.py
deepmd/descriptor/se_a.py
deepmd/fit/wfc.py
source/op/_prod_virial_se_a_grad.py
deepmd/entrypoints/test.py
deepmd/op/__init__.py
... and 76 more

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 843a3c5...c8ded77. Read the comment docs.

@denghuilu
Copy link
Member

Do we need to support the model compression workflow after skipping neighbor stat? For example, using training script as well as the training data to recompute the neighbor stat.

@njzjz
Copy link
Member Author

njzjz commented Nov 26, 2021

Do we need to support the model compression workflow after skipping neighbor stat? For example, using training script as well as the training data to recompute the neighbor stat.

We have already supported it...

@njzjz
Copy link
Member Author

njzjz commented Nov 26, 2021

Oh, currently one needs to provide the script even if it has been stored

@denghuilu
Copy link
Member

denghuilu commented Nov 27, 2021

Do we need to support the model compression workflow after skipping neighbor stat? For example, using training script as well as the training data to recompute the neighbor stat.

We have already supported it...

If there's no neighbor stat in the original training process, the model compression will not be abled due to the lack of min_nbor_stat. We need to recalculate min_nbor_stat in this situation.

The calculation of nborstat is necessary, either during training or during model compression.

@njzjz
Copy link
Member Author

njzjz commented Nov 27, 2021

Do we need to support the model compression workflow after skipping neighbor stat? For example, using training script as well as the training data to recompute the neighbor stat.

We have already supported it...

If there's no neighbor stat in the original training process, the model compression will not be abled due to the lack of min_nbor_stat. We need to recalculate min_nbor_stat in this situation.

The calculation of nborstat is necessary, either during training or during model compression.

t_min_nbor_dist = get_min_nbor_dist(jdata, get_rcut(jdata))

Isn't it?

@denghuilu
Copy link
Member

Do we need to support the model compression workflow after skipping neighbor stat? For example, using training script as well as the training data to recompute the neighbor stat.

We have already supported it...

If there's no neighbor stat in the original training process, the model compression will not be abled due to the lack of min_nbor_stat. We need to recalculate min_nbor_stat in this situation.
The calculation of nborstat is necessary, either during training or during model compression.

t_min_nbor_dist = get_min_nbor_dist(jdata, get_rcut(jdata))

Isn't it?

Do we need to support the model compression workflow after skipping neighbor stat? For example, using training script as well as the training data to recompute the neighbor stat.

We have already supported it...

If there's no neighbor stat in the original training process, the model compression will not be abled due to the lack of min_nbor_stat. We need to recalculate min_nbor_stat in this situation.
The calculation of nborstat is necessary, either during training or during model compression.

t_min_nbor_dist = get_min_nbor_dist(jdata, get_rcut(jdata))

Isn't it?

yes, no problem now

@wanghan-iapcm wanghan-iapcm merged commit 833c059 into deepmodeling:devel Nov 29, 2021
@njzjz njzjz deleted the skip-neighbor-stat branch December 21, 2021 23:40
njzjz added a commit to njzjz/deepmd-kit that referenced this pull request Sep 21, 2023
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.

long time before deepmd running
4 participants