Ticket #10702 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Create Workflow-algorithm for POLDI

Reported by: Michael Wedel Owned by: Michael Wedel
Priority: major Milestone: Release 3.4
Component: Diffraction Keywords: POLDI
Cc: Blocked By: #11285, #11616
Blocking: Tester: Federico Montesino Pouzols

Description

Now that more or less all algorithms necessary for POLDI are in place, I would like to create one or more Workflow-algorithms using these.

The first step is to check back with the beamline scientists which workflows are most important and most used and then implement them.

Change History

comment:1 Changed 6 years ago by Michael Wedel

Refs #10702. Small fix to PoldiMerge

Changeset: 603a8a6b18402625d7ce1c6e7daa2db459fc0e2c

comment:2 Changed 6 years ago by Michael Wedel

Refs #10702. Adding 2014f parameters for POLDI

Changeset: eec5a10a51a9a23a9ffcf63a0fb97b2a97f171da

comment:3 Changed 6 years ago by Michael Wedel

Refs #10702. Change naming of indexed peak workspaces.

Changeset: 382508a5488102a80d2fd0bc012d6dcaa2595986

comment:4 Changed 6 years ago by Michael Wedel

Refs #10702. First draft of workflow algorithm

Changeset: ef36cfee5f24f0986b466fca892dc1ea45356f57

comment:5 Changed 6 years ago by Michael Wedel

Refs #10702. Clang-formatting PoldiIndexKnownCompounds.cpp

Changeset: 6b116adddda5949352748741458c9d87838fa16f

comment:6 Changed 6 years ago by Nick Draper

  • Status changed from new to assigned

comment:7 Changed 6 years ago by Michael Wedel

  • Status changed from assigned to inprogress

comment:8 Changed 6 years ago by Michael Wedel

Refs #10702. Fixing small problem in PoldiFitPeaks1Dv2

Changeset: 0c334e1e0c9f229af147b22ddf626d41c9dbf0e6

comment:9 Changed 6 years ago by Michael Wedel

Refs #10702. Adjusting unit test of PoldiIndexKnownCompounds

Changeset: 06088106b72fb4c7e707c2b4239d2df8dc630396

comment:10 Changed 6 years ago by Michael Wedel

Refs #10702. Small changes to algorithm and formatting

Changeset: e4db272418d181bb3ac5ac479ff2ebb6ef886137

comment:11 Changed 6 years ago by Michael Wedel

Refs #10702. Addressing some pylint issues

Changeset: 1e0832490a22b4f963776535a5c8d1633ad9dbfd

comment:12 Changed 6 years ago by Michael Wedel

Refs #10702. Fixing typo.

Changeset: c025c411233c174ecf8634902c5e6ea5dc099b31

comment:13 Changed 6 years ago by Michael Wedel

  • Blocked By 11285 added

comment:14 Changed 6 years ago by Michael Wedel

Refs #10702. Checkpointing work.

Changeset: 24435d5a1d24e7a806b06a6f71b14960b757f8fd

comment:15 Changed 5 years ago by Michael Wedel

  • Blocked By 11616 added

comment:16 Changed 5 years ago by Michael Wedel

Refs #10702. Starting algorithm from scratch

The first approach to writing a workflow algorithm was too focused on one (complex) use case, which made it way to difficult to use the algorithm for more common cases. After further discussion with the scientists, the scope of the algorithm is better defined.

Changeset: de84c52e844a9624c85d13fa8ac637ed4f209d61

comment:17 Changed 5 years ago by Michael Wedel

Refs #10702. Using PointGroupFactory in PoldiPeakCollection

Changeset: 0521e7cf949bfc4d534b19eff3cbec026cdfa40d

comment:18 Changed 5 years ago by Michael Wedel

Refs #10702. Adding unit cell to PoldiPeakCollection

This will make InitialCell and CrystalSystem of PoldiFitPeaks2D obsolete, as PoldiIndexKnownCompounds now stores the cell and point group.

Changeset: 6c8f8616831b47fd8be413ed4a0e1d36a8038753

comment:19 Changed 5 years ago by Michael Wedel

Refs #10702. Change naming scheme of PoldiIndexKnownCompounds output

Previous naming scheme would overwrite indexed workspaces when using a compound multiple times.

Changeset: 48cced7483a815106c0ac23256eca258be637f61

comment:20 Changed 5 years ago by Michael Wedel

Refs #10702. Extracting crystal system names into PointGroup.h

This way it's easier to convert between strings/enum in different places.

Changeset: 9b35cf5ed55a19f4aa90b7392d2b5472a0cd646f

comment:21 Changed 5 years ago by Michael Wedel

Refs #10702. PoldiPeakCollection clone also unit cell and point group

Point group also has a default now, which is better than having a null pointer around.

Changeset: b1931e8043a0c72556f041c21028d57e85e5fc9a

comment:22 Changed 5 years ago by Michael Wedel

Refs #10702. Remove InitialCell and CrystalSystem from PoldiFitPeaks2D

These need to be present in the logs of the peak tables now, which they are when PoldiIndexKnownCompounds has been used to generate them. The docs contain a hint what to do when a custom indexing method has been used.

Changeset: 2447b05389f6f3941d498b242a8a36a4f6a7f819

comment:23 Changed 5 years ago by Michael Wedel

Refs #10702. Fixing typo in algorithm property doc

Changeset: 59341ab7fc6f3d36c1b3af0a5120f87f93c1c020

comment:24 Changed 5 years ago by Michael Wedel

Refs #10702. PoldiFitPeaks2D now accepts WorkspaceGroup for peaks

In this first step, workspace groups are accepted, but only the first one is used. Everything still works as expected.

Changeset: 351424c52ef8bcb5d3c6a43efd54fe683a088c5a

comment:25 Changed 5 years ago by Michael Wedel

Refs #10702. PoldiFitPeaks2D handles multiple phases

Changeset: 1f6f4e9317982dc5d76f932b0b898961b74af76e

comment:26 Changed 5 years ago by Michael Wedel

Refs #10702. Cleaning up order and comments in PoldiFitPeaks2D

Changeset: fe210d87c36de84b65f01a6886870b8e8045d2f4

comment:27 Changed 5 years ago by Michael Wedel

Refs #10702. Small fix to default point group in PoldiPeakCollection

Changeset: 104539529dcb5fb2b0d436b35d9875e5aff37386

comment:28 Changed 5 years ago by Michael Wedel

Refs #10702. Fixing system test for PoldiFitPeaks2D

Changeset: 88b6869654551e2f106f43aa3bd6fecb6d9fbae6

comment:29 Changed 5 years ago by Michael Wedel

Refs #10702. Declaring properties.

Changeset: c098bfaa347062c1dba9477f5e231a6a9e2df51a

comment:30 Changed 5 years ago by Michael Wedel

Refs #10702. Assign indices for all phases

Changeset: 90347b1b0c98d4767d55d3dea92292c865fd21d5

comment:31 Changed 5 years ago by Michael Wedel

Refs #10702. Small fix in PoldiFitPeaks1D

Returning the right workspace which has gone through multiple refinements, making peak criteria more strict.

Changeset: 17d859d7c0adfd80a03fae991d1753cd3a91caaa

comment:32 Changed 5 years ago by Michael Wedel

Refs #10702. Working algorithm.

Changeset: 813123322589e34097b7398caf1bd46d7887403a

comment:33 Changed 5 years ago by Michael Wedel

Refs #10702. Changing import of plotting

Changeset: 3fdad0a9c7b28e25df43bc5e2bc8ee2bdb510ed5

comment:34 Changed 5 years ago by Michael Wedel

Refs #10702. Added systemtest

Changeset: 6567f435f8e008bc385d2837ceb8390015637b0c

comment:35 Changed 5 years ago by Michael Wedel

Refs #10702. Added documentation for PoldiDataAnalysis.

Updated algorithm category as well.

Changeset: 62e965a5e9c9083d17be6bf63be5a44781e87d9c

comment:36 Changed 5 years ago by Michael Wedel

Refs #10702. Small fixes to doctests

As a result of the bug fixed in PoldiFitPeaks1D, one unindexed peak that is not actually a peak is excluded, so the numbers need to be adjusted by one.

Changeset: fc62c657a2752393e3c0ce0f071c3956ecf0d9f5

comment:37 Changed 5 years ago by Michael Wedel

Refs #10702. Correcting doc-test of PoldiDataAnalysis

Changeset: e55f4ce3b0ad38511bf9ff5b02cc73497ca46eb6

comment:38 Changed 5 years ago by Michael Wedel

Refs #10702. Small fix to PoldiCreatePeaksFromFile

Fixing space group symbol parsing (did not allow /) and allow rational numbers.

Changeset: 0d672978f2fe0db2311157ad9e6ddf2e058864b1

comment:39 Changed 5 years ago by Michael Wedel

Refs #10702. Added small output comment

Changeset: c9b1524e97ae88c4aaa2d7694a1e5be3246d6f88

comment:40 Changed 5 years ago by Michael Wedel

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

This is being verified as pull request #649.

comment:41 Changed 5 years ago by Michael Wedel

Refs #10702. Fixing system test after automatic masking

Changeset: c200ca182f9cf28a440bc11d9f7c1e24141c9e93

comment:42 Changed 5 years ago by Michael Wedel

Refs #10702. Fixing PawleyFit doctest

Changeset: 2db786db4991ac74db6d03778902cb6d05590ca3

comment:43 Changed 5 years ago by Andrei Savici

You should fix your pylint warnings:

  • for unused variables (say a=Load(filename), then unused a) just use dummy_variable_name instead
  • add newlines at the end of the files
  • add a statement to ignore variables defined outside init. Most python algorithms have this

comment:44 Changed 5 years ago by Michael Wedel

Refs #10702. Fixing pyling warnings

Changeset: c2ff80386f13a6e0c332052bf3bbb47dc15e6f6d

comment:45 Changed 5 years ago by Michael Wedel

Thanks Andrei, I missed the pylint warnings. But why was the indicator green?

comment:46 Changed 5 years ago by Andrei Savici

For now the threshold is set to 4000 warnings. We need to fix a few more, to slowly get it down

comment:47 Changed 5 years ago by Federico Montesino Pouzols

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

comment:48 Changed 5 years ago by Federico Montesino Pouzols

All looks good, and extensively documented and tested. I run the system and unit tests locally just in case, without surprises.

Maybe one small thing that could be added to the tests later on is some test cases with wrong properties (users mistype or forget a required property, etc.). And I am not sure if it is possible to have wrong combinations of properties (and/or property values), that could also be added in the unit tests.

I also have a general comment about unit tests, actually forwarding a comment from Owen some days ago on one of my PRs. The idea is to prefer the TSM_... versions of the CxxTest asserts rather than the TS_... adding informative messages wherever it makes sense, so that we can quickly get an idea of what's the issue when a unit test fails. Hum, but I see now that TSM is already used in many places already in these tests ;)

comment:49 Changed 5 years ago by Federico Montesino Pouzols

  • Status changed from verifying to closed

Merge pull request #649 from mantidproject/10702_poldi_workflow_algorithm

Add workflow algorithm for POLDI analysis

Full changeset: 435d4dab38a9a325ca472a1265f8953cd72ac362

comment:50 Changed 5 years ago by Michael Wedel

Thanks for testing thoroughly! You're right, I do not use TSM_ too often, I will pay more attention to this in the future. So far I tried to use them in "bulk" tests where the same check is run for similar data in the same test. Another thing I'm trying to work on is to make smaller, more specific test-cases, which makes it also easier to spot where an error originated, just from the name of the test-case.

comment:51 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 11544

Note: See TracTickets for help on using tickets.