Ticket #10281 (closed: fixed)
Create SpaceGroup class and factory
Reported by: | Michael Wedel | Owned by: | Michael Wedel |
---|---|---|---|
Priority: | major | Milestone: | Release 3.3 |
Component: | Framework | Keywords: | |
Cc: | anders.markvardsen@… | Blocked By: | #10280 |
Blocking: | #10262, #10283, #10344 | Tester: | Anders Markvardsen |
Description
As part of the changes in #10262, a new class SpaceGroup should be introduced. It will be constructed from a list of SymmetryOperation-objects (see also #10280), a space group number and the Hermann-Mauguin symbol. There will be two subclasses which implement different initialization methods. TabulatedSpaceGroup takes a complete list of symmetry operations for a space group, while GeneratedSpaceGroup takes a list of generators (also SymmetryOperation-objects) and generates all operations from that list. That way we don't need one class per space group.
SpaceGroup-instances will be constructed by SpaceGroupFactory, which also takes care of initialization of the objects. Because the initialization can be costly (either reading symmetry operations from a string, file or elsewhere or generating them), this will be done only once. Subsequent object creation will approximately follow the Prototype-pattern.
Currently the interface will be limited to generating all equivalent positions for a given coordinate vector V3D.
Change History
comment:4 Changed 6 years ago by Michael Wedel
- Status changed from assigned to inprogress
Refs #10281. Added Group and CyclicGroup
Changeset: 3b2e6faedaac9821e16d214009ca321b12b61a80
comment:5 Changed 6 years ago by Michael Wedel
Refs #10281. Removed counting variable from Group.
It was only used for testing purposes (counting number of required operations).
Changeset: d281ab24182a7b4a05dcc5bec24752b606ceb8c1
comment:6 Changed 6 years ago by Michael Wedel
Refs #10281. Added simple factory function for Group-objects.
Since groups can be constructed from strings only and are probably not used directly in code so much, this solution seems simpler than a complete factory.
Changeset: c2cbd2b5a9cbb780fde0ed0cf3f14186e0db3dcd
comment:7 Changed 6 years ago by Michael Wedel
Refs #10281. Creating vectors of symmetry operations.
Changeset: ce95b6742782920fa944b581aa5368b0badb0bae
comment:8 Changed 6 years ago by Michael Wedel
Refs #10280. Added class to deal with lattice centering
Changeset: 93c1bedec25ace8ef1c59547ea18bbaae041af62
(Accidentally put ticket #10280, sorry)
comment:9 Changed 6 years ago by Michael Wedel
Refs #10281. Moved helper class out of CenteringGroup
Changeset: d6d8875daf194b89e0c40c31d33e32e532af9c34
comment:10 Changed 6 years ago by Michael Wedel
Re #10281. Removing static members from SymmetryOperationSymbolParser
Changeset: 417b63577b95c504fd4b33f0733b5b9e4279645b
comment:11 Changed 6 years ago by Michael Wedel
Refs #10281. Added string constructor to Group.
Changeset: 87910525cd8746053db901100a058c3999397ca9
comment:12 Changed 6 years ago by Michael Wedel
Refs #10281. Added ProductGroup class
Changeset: e293a31cfb1beaf65250736d85f112c4f323127c
comment:13 Changed 6 years ago by Michael Wedel
Refs #10281. Added SpaceGroup class.
Changeset: 4d1072a27942917c17c70128c0b71a416de36e33
comment:14 Changed 6 years ago by Michael Wedel
Notes
- After starting implementation of groups and their relations I realized there is no need for having "GeneratedSpaceGroup" and "TabulatedSpaceGroup", this can be handled by implementing ProductGroup (will be useful for PointGroup as well) and offering two different registration modes in the factory.
- The factory needs to be a bit flexible concerning space group numbers, because often there are different settings for one group and they all occur under the same number. The Herrmann-Mauguin symbol is unique (appending "S" and "Z" if there are two origin choices) and is a better identifier for the groups in the factory.
- Currently I'm leaning towards solving this "problem" like that: The factory will return one space group when creation is performed via the symbol and a vector of space groups when the number is used.
comment:15 Changed 6 years ago by Michael Wedel
Refs #10281. Checkpointing SpaceGroupFactory.
Changeset: b67b10b230783fca273683d33fb4a0d730625e6f
comment:16 Changed 6 years ago by Michael Wedel
Refs #10281. Changed SpaceGroupFactory
Changeset: 42151b78810ac058a4e3947c1406d59b37ddf747
comment:17 Changed 6 years ago by Michael Wedel
Refs #10281. Updating documentation.
Changeset: 992f7fc8bdd9a264879f2d1964960ba62806c4c1
comment:18 Changed 6 years ago by Michael Wedel
Refs #10281. More work on SpaceGroupFactory.
Changeset: eed522ec4f2eca7d009d38b34ca23d8e02ca777d
comment:19 Changed 6 years ago by Michael Wedel
Refs #10281. Removing unused include.
Changeset: 7ee2b583df11f883f9a767b14c3c2e01532509cf
comment:20 Changed 6 years ago by Michael Wedel
Refs #10281. Removed static variable from CenteringGroupCreationHelper
Changeset: 95d3167b4f210e9784afb81f39feb6e587a373c0
comment:21 Changed 6 years ago by Michael Wedel
Refs #10281. Changed CenteringGroupCreationHelper again.
Changeset: 711e675fa753fd411ebfc0e2d395cf7b4c288cf0
comment:22 Changed 6 years ago by Michael Wedel
Refs #10281. Decided to change CenteringGroup helper once more
Having a singleton is better than having a static member variable which depends on initialization order. And it's better than to fill the symbol/type map every time a centering group is constructed.
Changeset: 589a3a86189d38e73582f6e243d03716500f26a4
comment:23 Changed 6 years ago by Michael Wedel
Refs #10281. Removing output generating unit tests
Changeset: 14ce7da805cef84f2955ccea74f870acc9d780ed
comment:24 Changed 6 years ago by Michael Wedel
Refs #10281. Added system test for space group factory.
The test generates all registered space groups and checks that their symmetry operations are correct.
Changeset: d58f7e6454ee9cdb525a2714bc82243a23d4c577
comment:25 Changed 6 years ago by Michael Wedel
Refs #10281. Exporting SpaceGroup and Factory to python
Changeset: fd637c9e7a815f287836fbe3bf609d02666f81c2
comment:26 Changed 6 years ago by Michael Wedel
Refs #10281. Changed system test.
Changeset: 11e842b747abad1e2c4d868e92998091918640e0
comment:27 Changed 6 years ago by Michael Wedel
Refs #10281. Added some space groups to the factory
Unfortunately I don't have time right now to add all of them, so I will make a new ticket for that.
Changeset: f3069b8dd7a4336106a94ece8054c26d1b64ef2d
comment:28 Changed 6 years ago by Michael Wedel
Refs #10281. Forgot const pointer export problem.
Changeset: 0dfe66e71d65b7996021ee4860bf94489bb781cb
comment:29 Changed 6 years ago by Michael Wedel
Notes
The goal of these ticket was to implement a SpaceGroup class that can be used to generate equivalent positions in the unit cell from the asymmetric unit. Example: A compound crystallizes in the space group Fm-3m and an atom is located at the position (0.21, 0.47, 0.86). Because of the high symmetry, this position is mapped to 192 equivalent positions in the unit cell - having to specify all these positions is very inconvenient.
A space group is a group of symmetry operations and applying all symmetry operations to the coordinates of the position results in all equivalent positions. Instead of reading this from a file, I chose to pursue the "group theoretical approach", creating the symmetry operations from a small set of generators. This way, it's very easy to refactor PointGroup later to fit into this scheme, take into account non-standard space group settings, perform transformations and so on.
The basis is provided by the class "Group", which most notably defines a multiplication operator with other groups. Then there is "CyclicGroup", which defines a group which can be expressed as powers of one generating symmetry operation. Another class "ProductGroup" automatically generates a cyclic group for each generator supplied to its constructor and multiplies these groups. Each space group can be generated by such a ProductGroup (using the generators from ITA) and the centering related symmetry operations (which are encapsulated in "CenteringGroup").
Space groups are registered in a factory, which uses the prototype pattern to minimize those generation efforts (in case of Fm-3m, which has 192 operations, it takes under 300 symmetry operation multiplications, this should be the worst case) to doing it only once - when the space group is subscribed into the factory. Creation is then performed like this:
SpaceGroup_const_sptr spaceGroup = SpaceGroupFactory::Instance().createSpaceGroup("F m -3 m");
SpaceGroup and SpaceGroupFactory are exported to python, but I only exported it to the degree so that it can be used in a system test(which checks the generated symmetry operations against reference data. All 230 space group types are in the reference data (but not all possible settings etc.), so this should cover the "first steps".
For my purposes, this is enough right, but I am willing to expand the code, as I started diving a bit deeper into this aspect of crystallographic computation and find it quite interesting.
Testing
- Code review
comment:30 Changed 6 years ago by Michael Wedel
- Status changed from inprogress to verify
- Resolution set to fixed
comment:32 Changed 6 years ago by Anders Markvardsen
- Status changed from verify to verifying
- Tester set to Anders Markvardsen
comment:33 Changed 6 years ago by Anders Markvardsen
Hi Michael,
Initially a bit surprised by the number of new classes, but looking in more details looks good.
I just have one class name I am not so happy with, if I understand you code correction, and that is ProductGroup. I would prefer you rename it to ProductCyclicGroup (Or ProductOfCyclicGroups or equivalent) since it is a group specific to hold a product of CyclicGroups. Is this OK? At the moment the name may lead a user to think that ProductGroup can be used to multiply any groups.
comment:34 Changed 6 years ago by Michael Wedel
Hi Anders,
thanks for testing - it turned out to be a bit bigger than I thought. After going through the literature a bit it seemed much easier building up the group things from the beginning. I think the math behind it comes out a bit more clear this way.
Looking at the class name again you are right. In the texts about point group generation this was called "product group", so I just went with that, but since the product is already implemented via operator* it makes sense to distinguish a bit.
Do you see anything else that needs to be changed?
comment:35 Changed 6 years ago by Anders Markvardsen
No, looks clear and well tested otherwise to me. Note you don't need to reopen the ticket in order to make further commits to it
comment:36 Changed 6 years ago by Michael Wedel
Refs #10281. Renamed ProductGroup to ProductOfCyclicGroups.
Following Anders' suggestion to name the class after what it actually does.
Changeset: 394251df3b0f6d395b63b8a89d38d6a217b4a38a
comment:37 Changed 6 years ago by Michael Wedel
Merge branch 'feature/10281_space_group_and_factory' into develop
Conflicts:
Code/Mantid/Framework/Geometry/CMakeLists.txt
Refs #10281
Changeset: 868e1679977484905b12856b8282b88b7d7f4e06
comment:38 Changed 6 years ago by Michael Wedel
Regarding the chat message from Martyn today: I added a system test to systemtests as well.
comment:39 Changed 6 years ago by Anders Markvardsen
Resolve merge conflict related to removed of tabs. re #10281
Merge remote-tracking branch 'origin/feature/10281_space_group_and_factory'
Conflicts:
Code/Mantid/Framework/Geometry/CMakeLists.txt
Changeset: 3c582585997265202bbacbb3cfa70b8bf17388cd
comment:40 Changed 6 years ago by Anders Markvardsen
- Status changed from verifying to closed
Hi,
Fixed trivial merged conflict and merged in systemtest also
comment:41 Changed 6 years ago by Anders Markvardsen
Fix bad merge conflict. re #10281
Changeset: 3ae9c14395f4bb21ecbed924df07c4209254a84d
comment:42 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 11123