updated with max_ent - #81
Conversation
There was a problem hiding this comment.
This should be a test in test_lib.py, not a whole new file
There was a problem hiding this comment.
Looks mostly good. but it looks like you're using an old version of tce-lib here.
After commit 7fb3ac3, we don't use scipy.spatial.KDTree anymore for the neighbor finding. Also, you re-implemented TCECalculator.get_batched_feature_vectors - this was added in the same commit.
| from opt_einsum import contract | ||
| from multiset import Multiset | ||
|
|
||
| # new import for optimized sorting: |
There was a problem hiding this comment.
The import order should look like:
- built-ins (standard library modules like
typing) - third party modules (like
numpy,sparse) - local modules (like
.training)
| LATIN_ALPHABET = "ijklmnopqrstuvwxyz" | ||
|
|
||
| #max_ent funct for optimized matrix sorting | ||
| def maximum_entropy_subset_up_to_size_k( |
There was a problem hiding this comment.
this should probably be a "private" method
i put private in quotes because python doesn't actually have these, but there's a standardized way we can denote it:
- rename it with an underscore (
foo -> _foo)
we also use pdoc to generate documentation and we shouldn't have private functions in the documentation. you can add @private in the doc-string to make sure pdoc doesn't catch it:
| topological_tensors = self.topological_tensors.get(topology_key) | ||
|
|
||
| if topological_tensors is None: | ||
|
|
There was a problem hiding this comment.
old tce-lib version, see the git diff where this error guard doesn't exist
| return obj | ||
|
|
||
| # err handling and center controlling | ||
| def get_batched_feature_vectors( |
There was a problem hiding this comment.
shouldn't exist, see comment on file
| """ | ||
| return np.array([self.get_feature_vector(atoms) for atoms in atoms_list]) | ||
|
|
||
| def select_maximum_entropy_subsets( |
There was a problem hiding this comment.
this should have a google-style doc-string as the rest of the functions in the library. This is so pdoc can automatically generate documentation
with the current version of tce-lib
2 participants
No description provided.