-
Notifications
You must be signed in to change notification settings - Fork 4
Feature/SOF-7782 Maxwell displacement #265
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
Conversation
src/py/mat3ra/made/periodic_table.py
Outdated
| if output_path is not None: | ||
| with open(output_path, "w") as f: | ||
| json.dump(data, f, indent=2) | ||
| return data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
| from mat3ra.made.utils import AXIS_TO_INDEX_MAP | ||
|
|
||
|
|
||
| class ElementalFunctionHolder(InMemoryEntityPydantic): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reuse FunctionHolder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AtomicMassDependentFunctionHolder(FunctionHolder)
| return displacement.tolist() | ||
|
|
||
|
|
||
| def create_maxwell_displacement_function( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be in helpers
| is_elemental = isinstance(perturbation_function, ElementalFunctionHolder) | ||
|
|
||
| for atom_index, coordinate in enumerate(original_coordinates): | ||
| if is_elemental: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
|
|
||
| for coordinate in original_coordinates: | ||
| # If func_holder returns a scalar, assume z-axis; otherwise vector | ||
| displacement = perturbation_function.apply_function(coordinate) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Send the element symbol together with this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Send the whole material with it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
additional_parameters = perturbation_function.calculate_additional_parameters(material)
in the loop:
displacement = perturbation_function.apply_function(coordinate=coordinate, additional_parameters=additional_parameters)
|
|
||
| super().__init__(function=function, variables=variables, **data) | ||
|
|
||
| def apply_function( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove atom_index
|
|
||
| if self.is_mass_used: | ||
| element = material.basis.elements.values[atom_index] | ||
| mass = get_atomic_mass_from_element(element) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can get all masses as array in the parent class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In material analyzer
| from .fixtures.bulk import BULK_Si_PRIMITIVE | ||
| from .fixtures.slab import SI_CONVENTIONAL_SLAB_001 | ||
|
|
||
| DISORDER_PARAMETER = 3000.0 # Temperature-like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove the conversion constant and just put a comment that DISORDER_PARAMETER is close to temperature in eV
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
including the effective "spring" constant next to the current state
| disorder_parameter: float, | ||
| random_seed: Optional[int] = None, | ||
| is_mass_used: bool = True, | ||
| conversion_constant: float = 2e-3, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be necessary anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes!
No description provided.