Skip to content
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

Orbit and function hierarchy #141

Merged
merged 9 commits into from
Oct 26, 2021
Merged

Orbit and function hierarchy #141

merged 9 commits into from
Oct 26, 2021

Conversation

lbluque
Copy link
Collaborator

@lbluque lbluque commented Sep 28, 2021

Summary

Implement methods to obtain orbit and correlation function hierarchy. Addresses issue #140

New methods in class Orbit:

  • is_sub_orbit will check if another orbit is a sub orbit of self.
  • sub_orbit_mapping will return a mapping from the sites in self to a given sub orbit.

New methods in class ClusterSubspace

  • orbit_hierarchy will give hierarchy as list of lists of orbit ids.
  • function_hierarchy will give a hierarchy of correlation functions (same format as above).
  • get_sub_orbits will return a list of orbits that are sub-orbits of the orbit id given.
  • get_sub_function_ids will return a list of corr function ids that are sub functions of the corr function for the given id.

TODO (if any)

  • This still needs robust unit-testing to make sure it works reliably for all systems.

Checklist

@lbluque
Copy link
Collaborator Author

lbluque commented Sep 28, 2021

@zhongpc and @qchempku2017 please have a look at this and consider trying to see how it behaves on some test systems when you get a chance. Let me know if you find issues, have suggestions, etc...

@lbluque lbluque marked this pull request as draft September 28, 2021 21:57
@lbluque
Copy link
Collaborator Author

lbluque commented Sep 28, 2021

I made a temporary feature branch hierarchy, for convenience in case it makes it easier to test/run the code.

I will delete it once we are ok with the fixes.

@zhongpc
Copy link
Contributor

zhongpc commented Sep 29, 2021

I check the new method, it's good.
The inconsistence comes from to find the cluster from sites in periodic lattice.

For example,
Let sites A B C in fractional site [[ 0. 1. 0. ] [-0.5 0.5 0.5] [ 1. 0. 0. ]]
distance from B-C is [1.5 0.5 0.5], however, this value excess the cutoff length.
The actual distance vector is [0.5 0.5 0.5], because the site C is on the equivalent site of [ 0. 0. 0. ].

As I reviewed, all the inconsistency comes from this type. The old method applies geometrically correct hierarchy, but in the bit enumeration process, there can by some error.
In the future work, please use subspace.function_hierarchy(min_size=xxx, invert = True/False) instead.

@qchempku2017
Copy link
Collaborator

I check the new method, it's good. The inconsistence comes from to find the cluster from sites in periodic lattice.

For example, Let sites A B C in fractional site [[ 0. 1. 0. ] [-0.5 0.5 0.5] [ 1. 0. 0. ]] distance from B-C is [1.5 0.5 0.5], however, this value excess the cutoff length. The actual distance vector is [0.5 0.5 0.5], because the site C is on the equivalent site of [ 0. 0. 0. ].

As I reviewed, all the inconsistency comes from this type. The old method applies geometrically correct hierarchy, but in the bit enumeration process, there can by some error. In the future work, please use subspace.function_hierarchy(min_size=xxx, invert = True/False) instead.

Hi Peichen,
I've checked Luis' code and your reply. The new implementation is quite need and should be much more efficient. But I still have a question on your reply.
What is that inconsistency you have mentioned? Where does it happen? Did you mean in the old version, the _orbits_from_cutoffs generates 160 clusters at 744 cutoff, instead of 161 clusters?

It looks like what you were talking about is a coordnate rounding issue, and in this new implementation, this like of np.mod treatment should have automatically solved your problem:

    for nbit, site, sbasis in zip(nbits, exp_struct, site_bases):
        # Coordinates of point terms must stay in [0, 1] to guarantee
        # correct math of the following algorithm.
        new_orbit = Orbit(
            [np.mod(site.frac_coords, 1)], exp_struct.lattice,
            [list(range(nbit))], [sbasis], symops)
        if new_orbit not in new_orbits:
            new_orbits.append(new_orbit)

True?

@lbluque lbluque marked this pull request as ready for review October 26, 2021 00:32
@lbluque lbluque changed the title [WIP] Orbit and function hierarchy Orbit and function hierarchy Oct 26, 2021
@lbluque lbluque merged commit 979279a into CederGroupHub:master Oct 26, 2021
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