Ticket #10504 (closed: fixed)
Change registration of SpaceGroups into Factory
Reported by: | Michael Wedel | Owned by: | Michael Wedel |
---|---|---|---|
Priority: | major | Milestone: | Release 3.3 |
Component: | Framework | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | #10344 | Tester: | Nick Draper |
Description (last modified by Michael Wedel) (diff)
The conclusion of a discussion about the growing number of factories in MantidGeometry was that this may lead to longer loading times when Mantid is launched, especially if registering objects requires a considerable amount of work.
- In the case of BraggScattererFactory there are only two registered classes and registering them does not require doing much (it's using Kernel::DynamicFactory).
- For PointGroup, the number of registered objects is small (13) and is ultimately limited to 32. Additionally, PointGroup may change in the not so far future (derive it from the Group-hierarchy, like SpaceGroup).
- SpaceGroupFactory is an issue, because there are hundreds of possible space groups that can be registered and currently a prototype is created when the class is registered - this is computationally expensive in high symmetry cases.
I want to change the way space groups are subscribed to the factory, postponing the creation of the prototype until a concrete group is requested. The first creation of a space group will then be slower compared to now, but a lot of potentially unnecessary work (if space groups are not used) is avoided on launching.
I would like to do this before #10344, where I plan to add many space groups.
Change History
comment:3 Changed 6 years ago by Michael Wedel
- Status changed from assigned to inprogress
Refs #10504. SpaceGroupFactory now generates prototypes on demand.
Changeset: 4c21f89c631f8b37cce60f5f82b40ff1a2bd2436
comment:4 Changed 6 years ago by Michael Wedel
Refs #10504. Extracted prototype generation into method
Changeset: 5f8e2a4f7c231e1df5e9c38dab2f5d92d9b46be4
comment:5 Changed 6 years ago by Michael Wedel
Refs #10504. Added AbstractSpaceGroupGenerator and two sub-classes
This will replace a lot of code in SpaceGroupFactory eventually.
Changeset: 49b71ee214143164af07263b5603a3debff00ae3
comment:6 Changed 6 years ago by Michael Wedel
Refs #10504. SpaceGroupFactory now uses AbstractSpaceGroupGenerator
The factory now uses AbstractSpaceGroupGenerator-objects to delay prototype construction until it's really needed. Separating the generation out of the factory itself makes it more flexible (other space group generation algorithm) and more focused (factory does not care anymore how prototypes are constructed).
Changeset: 9cdfac7beedc8a30c4baa8dcd2bca654d91a89b9
comment:7 Changed 6 years ago by Michael Wedel
Testing information
Code review. Make sure the SpaceGroupFactory system test passes (it has not been modified). The unit test has been expanded a bit - everything that used to work with the old registration method still works with the new method as well. There is also a new test case (using a mock-object) which makes sure that prototype generation is only done once.
comment:8 Changed 6 years ago by Michael Wedel
- Status changed from inprogress to verify
- Resolution set to fixed
comment:9 Changed 6 years ago by Nick Draper
- Status changed from verify to verifying
- Tester set to Nick Draper
comment:10 Changed 6 years ago by Nick Draper
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/feature/10504_improve_spacegroup_factory_registration'
Full changeset: 60cbda2ba65c65b6233563b8816984369b45bb3f
comment:11 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 11346