Ticket #11006 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Refactor PointGroup to use Group

Reported by: Michael Wedel Owned by: Michael Wedel
Priority: major Milestone: Release 3.4
Component: Framework Keywords: Geometry
Cc: Blocked By:
Blocking: Tester: Federico Montesino Pouzols

Description

PointGroup was created before the general group theoretical code for SpaceGroup was written. Refactoring the point group code would make the symmetry related code more coherent and also make it easier to extract the point group from the space group.

Furthermore it may make sense to replace PointGroup::isEquivalent(hkl1, hkl2) with a more general implementation that creates all equivalent reflections for hkl1 and checks whether hkl2 is contained in that list. This would also remove the need for having a separate class for each point group. On the other hand it may be slower than the current purely boolean logic based implementation.

Change History

comment:1 Changed 6 years ago by Michael Wedel

Refs #11006. Made SymmetryOperation matrix/vector constructor public

Changeset: 30587019884d8fc988e5b5d043e480fa6511ece2

comment:2 Changed 6 years ago by Nick Draper

  • Status changed from new to assigned

comment:3 Changed 6 years ago by Michael Wedel

  • Status changed from assigned to inprogress

Refs #11006. Added small performance test.

To address the concern added in the ticket description about possible performance decay for isEquivalent.

Changeset: 8dd731a22674c3593852a3fe770f9973b2c3a931

comment:4 Changed 6 years ago by Michael Wedel

Refs #11006. Changes point group to inherit from group

Changeset: d03774c76f6ab75b1f38d74eba6258c7aca8e422

comment:5 Changed 6 years ago by Michael Wedel

Refs #11006. Removing comments, performance improvements.

Changeset: 556d2d9b4e6ba0656f3a289d277a30a7cded10d6

comment:6 Changed 6 years ago by Michael Wedel

Refs #11006. Speeding up Kernel::Matrix * Kernel::V3D

Changeset: e30b34c76a2463eef0699ce253a0d6da639b41cf

comment:7 Changed 6 years ago by Michael Wedel

Refs #11006. Removing permutation helper

Changeset: 17398b97b979d3188057cad88d6041182bd005de

comment:8 Changed 6 years ago by Michael Wedel

Refs #11006. Making comparison in V3D more simple

Changeset: b84844a2dbf1f00a982b1b6c08b7441f7e3f0fbf

comment:9 Changed 6 years ago by Michael Wedel

Refs #11006. Using matrix only to generate hkls

Changeset: c0b138bfb16d1dbeeeee039f17aa421d66e60c2e

comment:10 Changed 6 years ago by Michael Wedel

Refs #11006. Fixing python export

Changeset: 8cd2cc3fd1eb7de45804fc9438cefe960f78a507

comment:11 Changed 6 years ago by Michael Wedel

Refs #11006. Compatibility with old code.

Changeset: 50af9fa3bbe0531d89c0c46b95e4156640c77aab

comment:12 Changed 6 years ago by Michael Wedel

Refs #11006. Fixing unit tests that depended on concrete point groups.

Changeset: 9cb66b035b44e8f4d7bd2f10490b1c05a7282bfb

comment:13 Changed 6 years ago by Michael Wedel

Refs #11006. Fixing small convention problem in axis determination

Changeset: 0de017d6c8acd09d0a776322ae5a94239dcba05d

comment:14 Changed 6 years ago by Michael Wedel

Refs #11006. Adding operator> to V3D

Changeset: 6f443d06a37bb3966518521159b83d473d65701c

comment:15 Changed 6 years ago by Michael Wedel

Refs #11006. Removed obsolete tests, add crystal system determination.

Changeset: 2dafbdd94a25aeaf3fb3a644b44c52a85c71c2a3

comment:16 Changed 6 years ago by Michael Wedel

Refs #11006. Fixing PointGroupFactoryTest

Changeset: abcd4bda0029346501797a91591d145dd1862fef

comment:17 Changed 6 years ago by Michael Wedel

Refs #11006. Fixing determination of tetragonal crystal system

Changeset: 69521a4591a183ce15e1f00fb3b182853d8f5cfc

comment:18 Changed 6 years ago by Michael Wedel

Refs #11006. Fixing determination of hexagonal crystal system

Changeset: 6ce32f734ed9d2d05559b90c1b26f1343007fa04

comment:19 Changed 6 years ago by Michael Wedel

Refs #11006. Removing comment

Changeset: 05cd0a1d5ea0e4499dac2acdb8b6c47c19c65dac

comment:20 Changed 6 years ago by Michael Wedel

Refs #11006. Adding all point groups.

Changeset: 5bbefef3a8e279f5fbd021ddb62d5ccf8495175f

comment:21 Changed 6 years ago by Michael Wedel

Refs #11006. Fixing doc test to include new point groups

Changeset: 98cd4af0fbe23e9490ae9fbe1f76a782b81e545a

comment:22 Changed 6 years ago by Michael Wedel

Refs #11006. Re-enabling additional point group tests.

Changeset: 100abb489cf0b4f22895a7b7a657a8cc90cf7b01

comment:23 Changed 6 years ago by Michael Wedel

Refs #11006. Fixing tests

Changeset: e42a9743e877f178c230d6584557e72624a54177

comment:24 Changed 6 years ago by Michael Wedel

comment:25 Changed 6 years ago by Michael Wedel

Refs #11006. Transposed matrices for hkl, corrected unit test.

Changeset: a9a71d4cc614942d181117bdca4c52f3380584ec

comment:26 Changed 6 years ago by Michael Wedel

Refs #11006. More cleaning

Changeset: fcb4a4ce503509ebb257f4cdc7e1d2bb1731fe08

comment:27 Changed 6 years ago by Michael Wedel

Refs #11006. Adding hexagonal crystal structure test.

Changeset: 1f6581a2dd60e5a23dd90e4c21019260908cb692

comment:28 Changed 6 years ago by Michael Wedel

  • Status changed from inprogress to verify
  • Resolution set to fixed

This is being verified as pull request #341.

comment:29 Changed 6 years ago by Michael Wedel

Refs #11006. Caught wrong exception type in python test.

Changeset: c8453c585894c5587651a43da6c9b1124f63d677

comment:30 Changed 6 years ago by Michael Wedel

Refs #11006. Changing point group name in SorHKL doctest

While the name in the doctest suggested that -3m using rhombohedral axis convention is used, the old implementation was in fact based on the hexagonal axis defintion.

Changeset: cc692e6cb5376553f685fb1f888572cd9b7adbcc

comment:31 Changed 6 years ago by Michael Wedel

Jenkins retest this please

comment:32 Changed 6 years ago by Michael Wedel

Jenkins retest this please

comment:33 Changed 6 years ago by Michael Wedel

Jenkins retest this please

comment:34 Changed 6 years ago by Michael Wedel

Refs #11006. Making matrix transpose for hkl clearer in code and docs.

Changeset: 69c089db0ea8065ba0f8c710d20be71f8268b8f0

comment:35 Changed 6 years ago by Michael Wedel

Jenkins retest this please

comment:36 Changed 6 years ago by Michael Wedel

Jenkins retest this please

comment:37 Changed 6 years ago by Michael Wedel

Jenkins retest this please

comment:38 Changed 6 years ago by Michael Wedel

Jenkins retest this please

comment:39 Changed 6 years ago by Michael Wedel

Refs #11006. Correcting HKL operations again

Found a reference for the different treatment of coordinates and vectors, implemented in SymmetryOperation, changed documentation. Added additional test for Kernel matrix operator.

Changeset: 5e4699b0750d8ba0574d253842acb0652aaf7e96

comment:40 Changed 6 years ago by Michael Wedel

Jenkins retest this please

comment:41 Changed 6 years ago by Federico Montesino Pouzols

  • Status changed from verify to verifying
  • Tester set to Federico Montesino Pouzols

comment:42 Changed 6 years ago by Federico Montesino Pouzols

Tests (unit, system, doc) passed locally, and on the CI servers doc tests are now run on win7, and system tests run on rhel6. So I'd take that as good sign enough. Also, the example and variations of it behave as expected. Code looks very good and comes with documentation and new test cases.

comment:43 Changed 6 years ago by Federico Montesino Pouzols

  • Status changed from verifying to closed

Merge pull request #341 from mantidproject/11006_refactor_point_group

Refactor point group to use Group

Full changeset: 7089106ad581b923b8d89d306ad10d9d9deef60a

comment:44 Changed 5 years ago by Nick Draper

Somehow these slipped through without a resolution. Set to Fixed.

comment:45 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 11845

Note: See TracTickets for help on using tickets.