add all_atomic_numbers flag
#10
Conversation
|
how about |
|
yes good idea. |
There was a problem hiding this comment.
Looks good in general. I just added one comment regarding the use of words like atomic "species" and atomic "type". Are we using "type" to also include isotopes and/or ions? For now, it seems like the code is mixing "type" and "species" (see comment) for some variable names.
I approved the changes since this is not such an urgent issue and goes beyond just MeshLODE, but it might be good to agree on how to homogenize this across our different softwares.
| @@ -129,18 +119,25 @@ def compute( | |||
| # of center_type and neighbor_type | |||
| blocks: List[TensorBlock] = [] | |||
| for keys, values in feat_dic.items(): | |||
| spec_center = atomic_numbers[keys // n_species] | |||
| spec_center = atomic_types[keys // n_species] | |||
There was a problem hiding this comment.
If we rename everything to "atomic_types", should we perhaps also rename spec_center to type_center etc. and homogenize the documentation to use "type" instead of "species"?
To be compatible with
torch_spexthis PR adds a new parameterall_atomic_numbersto set a global list of elements at initialization. If this is not set we use the "old" behavior and obtain the elements from the given systems.One question is how we want to name it. I followed the internal logic and named the parameter
all_atomic_numberseven though in torch_spex it is calledall_species.I also did some minor docs layout work.
📚 Documentation preview 📚: https://meshlode--10.org.readthedocs.build/en/10/