Conversation
rosecers
left a comment
rosecers
left a comment
There was a problem hiding this comment.
Hi @YCC-ProjBackups -- love this. Thank you for correcting my bad spelling.
All looks good but with some questions -- why are we adding new imports to projection coefficients?
curiosity54
left a comment
curiosity54
left a comment
There was a problem hiding this comment.
just wanted to make sure all the imports are okay
| import numpy as np | ||
|
|
||
| from ..representations.radial_basis import RadialBasis | ||
| from ..utils import quaternion_to_rotation_matrix # missing? |
There was a problem hiding this comment.
Is this really missing?
There was a problem hiding this comment.
Yes, everything still runs fine.
For the quaternion_to_rotation_matrix, I am fairly certain it is missing. The reason for not crashing the program is probably because we never use DensityProjectionClass calculator -- only EllipsoidalDensityProjection. In EllipsoidalDensityProjection, we use scipy.spatial.transform.Rotation.from_quat(...) to convert quaternion to rotation type.
Please let me know if I missed anything!
There was a problem hiding this comment.
I'm fine with removing it if it's no longer needed. It might be a relic from the first version.
There was a problem hiding this comment.
yep, we can remove this line in that case
Replying to @rosecers, I have not added any imports (only removed unused ones) -- Github probably shows it as addition due to changed ordering of the imports. Let me know if you still have questions! |
| import numpy as np | ||
|
|
||
| from ..representations.radial_basis import RadialBasis | ||
| from ..utils import quaternion_to_rotation_matrix # missing? |
There was a problem hiding this comment.
| from ..utils import quaternion_to_rotation_matrix # missing? |
| radial_hypers = { | ||
| "radial_gaussian_width": radial_gaussian_width, | ||
| } | ||
| self.radial_basis = RadialBasis( | ||
| radial_basis_name.lower(), max_angular, **radial_hypers | ||
| ) |
There was a problem hiding this comment.
| radial_hypers = { | |
| "radial_gaussian_width": radial_gaussian_width, | |
| } | |
| self.radial_basis = RadialBasis( | |
| radial_basis_name.lower(), max_angular, **radial_hypers | |
| ) | |
| radial_hypers = {} | |
| radial_hypers["radial_basis"] = radial_basis_name.lower() # lower case | |
| radial_hypers["radial_gaussian_width"] = radial_gaussian_width | |
| radial_hypers["max_angular"] = max_angular | |
| self.radial_basis = RadialBasis(**radial_hypers) |
3 participants
I went through all files inside anisoap folder and deleted unused variables and imports.
Additionally, some typos in the comments are fixed.
I created a new branch from the most recent main and avoided making any breaking changes, so everything should work the same way still.