feature/SOF 7413 #154
Merged
feature/SOF 7413 #154
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
VsevolodX
commented
Aug 15, 2024
VsevolodX
commented
- feat: first implementation of a nanrobibon
- update: corrections
- chore: run lint fix
- update: wrap in a builder
- update: make work + docstring
- update: make work correctly
- update: fix for even number fo vacuum{
- update: armchair case
- feat: add undercoordiantein analysis
- feat: add passivation modfy:
- update: add atom to material
src/py/mat3ra/made/tools/analyze.py
Outdated
| @@ -265,13 +266,15 @@ def get_atom_indices_with_condition_on_coordinates( | |||
| def get_nearest_neighbors_atom_indices( | |||
| material: Material, | |||
| coordinate: Optional[List[float]] = None, | |||
| cutoff: float = 15.0, | |||
src/py/mat3ra/made/tools/analyze.py
Outdated
| try: | ||
| neighbors_indices = get_nearest_neighbors_atom_indices(material, coordinate, cutoff=3) | ||
| except: | ||
| print("error") |
src/py/mat3ra/made/tools/modify.py
Outdated
| undercoordinated_atom_indices = get_undercoordinated_atom_indices(passivated_slab) | ||
| for i in undercoordinated_atom_indices: | ||
| atom_coordinate = passivated_slab.basis.coordinates.values[i] | ||
| # TODO: change normal to be from the cloeses center of mass |
src/py/mat3ra/made/tools/modify.py
Outdated
| @@ -437,3 +439,15 @@ def rotate_material(material: Material, axis: List[int], angle: float) -> Materi | |||
| atoms.wrap() | |||
|
|
|||
| return Material(from_ase(atoms)) | |||
|
|
|||
|
|
|||
| def passivate_surface(slab: Material, passivant: str, bond_length: float = 3.0): | |||
There was a problem hiding this comment.
Needs to be close to slab
src/py/mat3ra/made/basis.py
Outdated
| @@ -76,7 +76,17 @@ def to_crystal(self): | |||
| self.coordinates.map_array_in_place(self.cell.convert_point_to_crystal) | |||
| self.units = AtomicCoordinateUnits.crystal | |||
|
|
|||
| def add_atom(self, element="Si", coordinate=[0.5, 0.5, 0.5]): | |||
| def add_atom(self, element="Si", coordinate=None, force=False): | |||
There was a problem hiding this comment.
The function signature here should be aware of whether the units of the coordinate are cartesian or crystal. We can add use_cartesian or something like that.
Before lines 90 and 91 we need to check and confirm that the current units are compatible - i.e. if units are cartesian, we should add cartesian_coordinate, otherwise - crystal
src/py/mat3ra/made/tools/analyze.py
Outdated
| if site_index is None: | ||
| structure.append("X", coordinate, validate_proximity=False) | ||
| site_index = len(structure.sites) - 1 | ||
|
|
src/py/mat3ra/made/tools/analyze.py
Outdated
| return (z >= z_extremum - depth) if surface == SurfaceTypes.TOP else (z <= z_extremum + depth) | ||
|
|
||
|
|
||
| def shadowing_check(z: float, neighbors_indices: List[int], surface: SurfaceTypes, coordinates: np.ndarray) -> bool: |
| def condition(coordinate: List[float]): | ||
| return True | ||
|
|
||
| thickness_nudge_value = (added_layers_max_z - original_max_z) / thickness |
There was a problem hiding this comment.
what is this thickness_nudge_value?? Is there a better name? If not - explain in a comment
There was a problem hiding this comment.
thickness_nudge_value_z + comment
There was a problem hiding this comment.
I'll switch to cartesians and do this the proper way. Wasn't clear at the time
| @@ -0,0 +1,7 @@ | |||
| # TODO: Get this from periodic table | |||
|
|
|||
| BOND_LENGTHS_MAP = { | |||
There was a problem hiding this comment.
This doesn't seem to be used anywhere
VsevolodX
commented
Sep 10, 2024
| # Filter atoms in the added layers | ||
| island_material = filter_by_box( | ||
| material=atoms_within_island, | ||
| min_coordinate=[0, 0, original_max_z], | ||
| min_coordinate=[0, 0, original_max_z - thickness_nudge_value], |
There was a problem hiding this comment.
Add a comment to explain what is happening
…reapncies between Python and Pyodide results
| coordination_numbers = set( | ||
| get_coordination_numbers(material=material, cutoff=self.build_parameters.shadowing_radius) | ||
| ) | ||
| print("coordination numbers:", coordination_numbers) |
There was a problem hiding this comment.
- Let's name this function
get_unique_coordination_numbers - Update the description to state accordingly
- Remove the print statement
timurbazhirov
approved these changes
Sep 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.