Ticket #10280 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Extend SymmetryOperation to handle translational components

Reported by: Michael Wedel Owned by: Michael Wedel
Priority: major Milestone: Release 3.3
Component: Framework Keywords:
Cc: anders.markvardsen@… Blocked By:
Blocking: #10262, #10281, #10305 Tester: Anders Markvardsen

Description

As a first step for #10262, SymmetryOperation needs to be changed to be able to handle translational components. In that context, the factory will also be adapted, so that symmetry operations may be constructed from coordinate triplets in the form of [x, y, z] (I found the term "Jones faithful representation" for that notation). As in the International Tables, shifts should be given as fractions, "1/4", "2/3", etc.

Change History

comment:1 Changed 6 years ago by Michael Wedel

  • Blocking 10281 added

comment:2 Changed 6 years ago by Anders Markvardsen

  • Status changed from new to assigned

comment:3 Changed 6 years ago by Michael Wedel

  • Status changed from assigned to inprogress

Refs #10280. Added class for rational number vector

It will be useful for implementing the translational part of SymmetryOperation, because values like 2/3 etc. occur and floating point math just makes comparisons etc. harder.

Changeset: 148a11d85da16636154da72d1fcf4fa921244ab1

comment:4 Changed 6 years ago by Michael Wedel

Refs #10280. Started writing unit tests for V3R

Changeset: e37aa458e8fca86b59d7168ada5c8f468cf60c02

comment:5 Changed 6 years ago by Michael Wedel

Refs #10280. Changed default constructor

Changeset: 29bc755d4980e9a1613428d45cbe6ebc8061ece9

comment:6 Changed 6 years ago by Michael Wedel

Refs #10280. Finished unit tests, added documentation for V3R.

Changeset: 1a908bc0512d3611729514aa726bab2f911484df

comment:7 Changed 6 years ago by Michael Wedel

Refs #10280. Added parser for SymmetryOperation from [x,y,z] format.

Changeset: 79bd522fce3458e88863bc239c0ab0f2c2223b7c

comment:8 Changed 6 years ago by Michael Wedel

Refs #10280. Added V3R constructor from integer vector

Changeset: 71c37e66df9e86c1c1eb08ce1b0486a713d8b51a

comment:9 Changed 6 years ago by Michael Wedel

Refs #10280. SymmetryOperation identifier is derived from components

Internal matrix and vector are used to construct the identifier. This way it will be easier to store unique SymmetryOperation objects in the factory.

Changeset: 0bb0f665823e9d490bc452843a707352051b06c8

comment:10 Changed 6 years ago by Michael Wedel

Refs #10280. Changed order of generated identifier.

Changeset: 6007415f237148f0c56c275c1655ee9c0f8eb29f

comment:11 Changed 6 years ago by Michael Wedel

Refs #10280. Added symbol parser into own class.

Changeset: fd73dd29d388e842596be9d760e6f6598ccd11ed

comment:12 Changed 6 years ago by Michael Wedel

Refs #10280. Refactored SymmetryOperation and factory

Changeset: 55af992bc77256630949d5b7f43948f1efb7fd70

comment:13 Changed 6 years ago by Michael Wedel

Refs #10280. Changed python exports

Changeset: 0bb4520f77114e8e4a4676a73ef189788a607510

comment:14 Changed 6 years ago by Michael Wedel

Refs #10280. Updated documentation comments.

Changeset: d0c46d61266c66ad4c6c76f4b3aaa1dc218971ac

comment:15 Changed 6 years ago by Michael Wedel

Refs #10280. Added query method to SymmetryOperationFactory

Changeset: 396826380b7d278b809d1be9bcc7243bff428691

comment:16 Changed 6 years ago by Michael Wedel

Refs #10280. Cleaned up SymmetryOperation.

Changeset: c6964888a22dff2b7141527c240a7bdc7fab3323

comment:17 Changed 6 years ago by Michael Wedel

Refs #10280. Updated documentation.

Changeset: 204fbaac91980ff06d2e0e246cf60678d9e43629

comment:18 Changed 6 years ago by Michael Wedel

Refs #10280. Removed constructor-chaining from SymmetryOperation.

Changeset: 160a9a1a38d8aee222a97f37b15eb0dc3035fb70

comment:19 Changed 6 years ago by Michael Wedel

Refs #10280. Catching exceptions by reference.

Changeset: 330ee9c1b67b626b01ca5f78cc6f03420c9a6787

comment:20 Changed 6 years ago by Michael Wedel

  • Status changed from inprogress to verify
  • Cc anders.markvardsen@… added
  • Resolution set to fixed

Overview

  • SymmetryOperation has been changed to handle translational symmetry (using a matrix/vector pair).
  • Instead of having one class for each symmetry operation I wrote a parser that parses the Jones faithful symbol and constructs a matrix/vector pair.
  • To be able to carry out operations exactly (will be important in #10281 for space group generation), I added a new vector class V3R (similar to Kernel::V3D), which is a vector of three rational numbers (using boost::rational).
  • The factory is still there, because once an operation has been constructed, that SymmetryOperation object is stored as a prototype. Subsequent constructions are then only copies of that prototype, which is much faster than parsing the symbol each time (constructing the same operation 1 million times was 0.2 vs. 7.75 seconds on my computer - the parser is probably way too slow, but nothing depends on its specifics, so it is replacable).
  • As a consequence, the notation for symmetry operations I chose previously is gone now.
  • This change also affects PointGroup, which now directly stores symmetry operation objects instead of Kernel::IntMatrix, preparing for later changes.
  • The "Point groups" concept page has been adapted to these changes.

Testing

  • Code review
  • Make sure all tests pass (checkbuild passed, so that should be okay).
Last edited 6 years ago by Michael Wedel (previous) (diff)

comment:21 Changed 6 years ago by Michael Wedel

Refs #10280. Fixing uninitialized value error.

Changeset: 1f5352c3a5f784eac19a92732fc6e8e032c53668

comment:22 Changed 6 years ago by Michael Wedel

  • Status changed from verify to reopened
  • Resolution fixed deleted

comment:23 Changed 6 years ago by Michael Wedel

  • Status changed from reopened to inprogress

Refs #10280. Added inverse() and less than operator.

Changeset: 6f7873cbfb4d4882a4f36cf4c8fc33142711fd0c

comment:24 Changed 6 years ago by Michael Wedel

  • Blocking 10305 added

comment:25 Changed 6 years ago by Michael Wedel

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

Ticket #10305 will deal with determining symmetry elements from SymmetryOperation, so I'm marking this ticket fixed.

comment:26 Changed 6 years ago by Michael Wedel

Accidentally put this ticket number into a commit for #10281, sorry.

Last edited 6 years ago by Michael Wedel (previous) (diff)

comment:27 Changed 6 years ago by Anders Markvardsen

  • Status changed from verify to verifying
  • Tester set to Anders Markvardsen

comment:28 Changed 6 years ago by Anders Markvardsen

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/10280_introduce_translational_symmetry_operations'

Full changeset: 1c9879fc626b2847c1a067de30336df17a3ecc02

comment:29 Changed 6 years ago by Anders Markvardsen

Considered name V3R which I think goes well with V3D for in this case storing rational numbers.

Well documented.

One comment. When running

{ from mantid.geometry import SymmetryOperation, SymmetryOperationFactoryImpl symOp = SymmetryOperationFactoryImpl.Instance().createSymOp("x,y,-z+1/2") hkl = [1, -1, 3] hklPrime = symOp.apply(hkl) }

hklPrime is returned as V3D. One optional think to consider at some point is if hkl is inputted as a V3R (or V3R like) a V3R is returned. Discussed this with Martyn, and if done should consider how to make this transparent to python user

comment:30 Changed 6 years ago by Michael Wedel

Thanks for your comments. I finally figured out that I had to configure an e-mail address in order to receive notifications - better late than never :)

I interpreted "V3D" as "vector of three doubles", so "V3R" seemed like a logical choice. Also, it's short, which is convenient.

Better python integration is a good point, but I'm not sure about the best way to go here. I think it's somehow a small conceptional problem, because there are actually two things here: Applying symmetry operations to a vector, which only uses the matrix-part and applying symmetry operations to a point, which also add the translation vector.

HKLs are vectors, so adding translational parts makes no sense. I guess if these two things were conceptually divided (along the lines of having V3D as a vector and P3D as a point), it could be easier to express this in code as well. The same would apply for V3R (and V3I, which would be okay to use as HKL in most cases except modulated structures as far as I know).

The differences are small, but there's no way the operator can now whether it's processing a vector or a point when both are represented by the same data structure.

comment:31 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 11122

Note: See TracTickets for help on using tickets.