Finish model compression for se_r descriptor! #1361
Conversation
deepmd/descriptor/se_a.py
Outdated
| @@ -120,8 +120,6 @@ def __init__ (self, | |||
| """ | |||
| Constructor | |||
| """ | |||
| if rcut < rcut_smth: | |||
| raise RuntimeError("rcut_smth (%f) should be no more than rcut (%f)!" % (rcut_smth, rcut)) | |||
There was a problem hiding this comment.
Why did you delete these two lines?
deepmd/descriptor/se_r.py
Outdated
| if rcut < rcut_smth: | ||
| raise RuntimeError("rcut_smth (%f) should be no more than rcut (%f)!" % (rcut_smth, rcut)) |
There was a problem hiding this comment.
Sorry for that. This may be because I used the previous version of code when committing mine. I will recall it.
deepmd/utils/tabulate.py
Outdated
| except Exception: | ||
| self.sel_a = self.graph.get_operation_by_name('DescrptSeR').get_attr('sel') | ||
| self.prod_env_mat_op = self.graph.get_operation_by_name ('DescrptSeR') | ||
| else: |
There was a problem hiding this comment.
elif isinstance(self.descrpt, deepmd.descriptor.DescrptSeA): is recommended.
And the conditions stop when else which capture the case self.descrpt is neither DescrptSeA nor DescrptSeR, and throw an error.
There was a problem hiding this comment.
Yes, it‘s safer. I will change it~
deepmd/utils/tabulate.py
Outdated
| try: | ||
| self.sel_a = self.graph.get_operation_by_name('ProdEnvMatR').get_attr('sel') | ||
| self.prod_env_mat_op = self.graph.get_operation_by_name ('ProdEnvMatR') | ||
| except Exception: |
There was a problem hiding this comment.
Use KeyError instead of Exception. https://www.tensorflow.org/api_docs/python/tf/Graph#get_operation_by_name gives what get_operation_by_name raises.
deepmd/descriptor/se_r.py
Outdated
|
|
||
| for ii in range(len(self.filter_neuron) - 1): | ||
| if self.filter_neuron[ii] * 2 != self.filter_neuron[ii + 1]: | ||
| raise RecursionError( |
There was a problem hiding this comment.
Why is it RecursionError? I think it should be NotImplementedError instead.
|
Could you also update https://docs.deepmodeling.org/projects/deepmd/en/latest/freeze/compress.html? |
Sure~ |
Codecov Report
@@ Coverage Diff @@
## devel #1361 +/- ##
==========================================
+ Coverage 75.63% 75.70% +0.07%
==========================================
Files 92 92
Lines 7531 7628 +97
==========================================
+ Hits 5696 5775 +79
- Misses 1835 1853 +18
Continue to review full report at Codecov.
|
| node = self.embedding_net_nodes[f"filter_type_{ii // self.ntypes}{self.suffix}/matrix_{layer}_{ii % self.ntypes}"] | ||
| matrix["layer_" + str(layer)].append(tf.make_ndarray(node)) | ||
| else: | ||
| matrix["layer_" + str(layer)].append(np.array([])) |
There was a problem hiding this comment.
Please add an else to raise the error of un-supported descritpor
| node = self.embedding_net_nodes[f"filter_type_{ii // self.ntypes}{self.suffix}/bias_{layer}_{ii % self.ntypes}"] | ||
| bias["layer_" + str(layer)].append(tf.make_ndarray(node)) | ||
| else: | ||
| bias["layer_" + str(layer)].append(np.array([])) |
There was a problem hiding this comment.
Please add an else to raise the error of un-supported descritpor
| @@ -317,6 +379,9 @@ def _get_env_mat_range(self, | |||
| var = np.square(sw / (min_nbor_dist * self.dstd[:, 1:4])) | |||
| lower = np.min(-var) | |||
| upper = np.max(var) | |||
| elif isinstance(self.descrpt, deepmd.descriptor.DescrptSeR): | |||
| lower = np.min(-self.davg[:, 0] / self.dstd[:, 0]) | |||
| upper = np.max(((1 / min_nbor_dist) * sw - self.davg[:, 0]) / self.dstd[:, 0]) | |||
There was a problem hiding this comment.
Please add an else to raise the error of un-supported descritpor
| elif isinstance(self.descrpt, deepmd.descriptor.DescrptSeR): | ||
| layer_size = len(self.embedding_net_nodes) // ((self.ntypes * self.ntypes - len(self.exclude_types)) * 2) | ||
| if self.type_one_side : | ||
| layer_size = len(self.embedding_net_nodes) // (self.ntypes * 2) | ||
| return layer_size |
There was a problem hiding this comment.
Please add an else to raise the error of un-supported descritpor
| elif isinstance(self.descrpt, deepmd.descriptor.DescrptSeR): | ||
| table_size = self.ntypes * self.ntypes | ||
| if self.type_one_side : | ||
| table_size = self.ntypes |
There was a problem hiding this comment.
Please add an else to raise the error of un-supported descritpor
| except KeyError: | ||
| self.sel_a = self.graph.get_operation_by_name('DescrptSeA').get_attr('sel_a') | ||
| self.prod_env_mat_op = self.graph.get_operation_by_name ('DescrptSeA') | ||
| elif isinstance(self.descrpt, deepmd.descriptor.DescrptSeT): |
There was a problem hiding this comment.
I think DescrptSeA and DescrptSeT are the same here?
There was a problem hiding this comment.
Yes, they are the same. But considering the possible changes later, I have explicitly distinguished them.
OK. I will add an UT for this. |
There was a problem hiding this comment.
It is very nice to see the new python tests checking the output of the original model is identical to the compressed model.
Could you add the unittest for the new methods tabulate_fusion_se_r_*_cpu and tabulate_fusion_se_r_*_gpu_cuda added in source/lib/include/tabulate.h by this PR?
You may take as examples the tests of tabulate_fusion_se_a_*_cpu and tabulate_fusion_se_a_*_gpu_cuda in source/lib/tests/test_tabulate.cc
|
OK. I will add them. |
|
Your UT failed. Please check. |
|
https://github.com/deepmodeling/deepmd-kit/pull/1391/files#r781755472 A mistake in #1391 caused this. It couldn't pass UT but it was merged into devel. |
Now model compression is available for se_r descriptor.
The compressed result has been checked with lammps , both on CPU and GPU. It's correct and the deviation is within 1e-8 in 1000 steps.
Acceleration performance: CPU 300%, GPU 120% (may be due to the small test system).