Skip to content

Support MIC and properly normalise site vector#191

Open
GabrielBram wants to merge 4 commits intomainfrom
rotation_bug_fix
Open

Support MIC and properly normalise site vector#191
GabrielBram wants to merge 4 commits intomainfrom
rotation_bug_fix

Conversation

@GabrielBram
Copy link
Collaborator

Implement support for a site with neighbours that stretch across periodic boundaries. Also fixed non-normalised vector for lone pairs > 1.

@GabrielBram GabrielBram requested a review from robinsonmt1 March 25, 2025 08:53
Copy link
Collaborator

@robinsonmt1 robinsonmt1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds in check for minimum image when finding vectors to each neighbour atom, so should now work for atoms on the edge of a unit cell.

The placement and rotation should also still work even if the adsorbate is outside of the unit cell (previously still worked when the adsorbate was big enough to stretch outside of the unit cell even if the adsorption site was necessarily inside the cell)

rot_matrix = self.normal_rotation_matrix(theta, z_rot)

site_norm = np.dot(rot_matrix, site_norm.T).T
site_norm = site_norm / np.linalg.norm(site_norm)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Causes failure of the CI tests. Without this line, neighbour lists and site_norms are consistent between cutoff_mult values. When this line is included, the neighbour lists are inconsistent, with cutoff_mult=1 missing the Si atom (atom index: 2) and cutoff_mult=1.2 producing a different site_norm with a length of 0.9999999963586863 (inc. norm) vs 1.0000000034771082 (not inc. norm).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The site_norm difference is fine, because of the precision of positions in ASE up to 1e-8. The error comes from the ~6e-9 difference, giving positions with a total error of only 0.276. This will just require changing the computed positions to the new ones with the normalised site_norm

@robinsonmt1
Copy link
Collaborator

Currently I can't work out why the neighbours_list gives what it does for the 1th shell in the MeOH placing (shell around the O atom), even without using the cutoff_mult.

Ideally, after the H is added, the MeOH should be added directly above the H (O-H-O = 180[deg]). The following is the behaviour and stats for cutoff_mult values from 0.9 - 1.1:
(Distances: O-Al = 1.61 A, O-Si = 1.61 A)
cutoff_mult = 0.9: correct behaviour, only includes atom 0 in the 1th shell (cutoffs: O = 0.594, Al = 1.089, Si = 0.999)
cutoff_mult = 1: incorrect behaviour, includes atom 1 (Al) in the 1th shell, giving an incorrect site_norm vector (cutoffs: O = 0.66, Al = 1.21, Si = 1.11)
cutoff_mult = 1.1: slightly incorrect behaviour, includes both atoms 1 (Al) and 2 (Si) in the 1th shell, partially cancelling out and giving a near-correct site, but with an O-H-O angle of 170.5[deg]. (cutoffs: O = 0.726, Al = 1.331, Si = 1.221)

My issue with these is that if the neighbours function does use the covalent radii/natural_cutoffs, then it seems like the Si atom should also be included in the cutoff_mult=1 and the Al atom should still be included even in cutoff_mult=0.9. I think I must be missing or miscalculating something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants