Group-subgroup relationship bugs in SpaceGroup and PointGroup class

[kaueltzen]

Python version

3.10

Pymatgen version

master

Operating system version

No response

Current behavior

Hey, two remarks regarding the is_subgroup method of SpaceGroup:

First, the current implementation does not allow isomorphic group-subgroup relationships (same space group type) because of

pymatgen/src/pymatgen/symmetry/groups.py

Line 537 in 0234182

new_sub_groups.update([j for j in max_subgroups[i] if j not in all_groups])

For example, I would expect the following code to not raise an error:
assert SpaceGroup("P3").is_subgroup(SpaceGroup("P3"))
The if j not in all_groups needs to be removed in the first iteration.

Second, the current implementation disregards klassengleiche group-supergroup relationships where the overall translational symmetry is increased, but not through centering. For example, I would expect this to not raise an error:
assert SpaceGroup("Fm-3m").is_subgroup(SpaceGroup("Pm-3m"))

However, because of

pymatgen/src/pymatgen/symmetry/groups.py

Line 528 in 0234182

if len(supergroup.symmetry_ops) < len(self.symmetry_ops):

this returns False. I assume it is to save resources? I would either remove it completely (safest) or restrict it to certain cases, if possible(?).

---

Also, I had a look at the uncommented PointGroup subgroup test:

pymatgen/tests/symmetry/test_groups.py

Line 59 in 0234182

# assert pg3m.is_subgroup(pgm3m)

This test is failing because the -3 is defined along (111) in m-3m, but along (001) in -3m (therefore, the matrices are different). Generally, this affects all group-subgroup relationships if the groups are in crystal systems with different crystallographic directions / blickrichtungen. For example, this is also failing,
assert PointGroup("2").is_subgroup(PointGroup("4"))
because the rotation axis is along c in the tetragonal, but is notated along b in the monoclinic system (in this database).
I would specify this in the warning

pymatgen/src/pymatgen/symmetry/groups.py

Line 79 in 0234182

warnings.warn("This is not fully functional. Only trivial subsets are tested right now. ")

In the future, it would probably be best to have a database similar to the maximal subgroup database of space group types.

---

If you agree, I can implement and test the SpaceGroup changes.

Expected Behavior

I would expect my code snippets from above to not raise an error.

Minimal example

No response

Relevant files to reproduce this bug

No response