Ticket #11285 (closed: fixed)
Create algorithm that loads, truncates, checks and merges a range of POLDI files
Reported by: | Michael Wedel | Owned by: | Michael Wedel |
---|---|---|---|
Priority: | major | Milestone: | Release 3.4 |
Component: | Diffraction | Keywords: | POLDI |
Cc: | Blocked By: | ||
Blocking: | #10702 | Tester: | Federico Montesino Pouzols |
Description
Loading POLDI data and bringing them into appropriate form for further processing takes a few steps:
- LoadSINQ
- LoadInstrument
- PoldiTruncateData
- Optionally PoldiMerge for measurements with more than one data file
I would like to combine these steps into one algorithm that takes the following parameters:
- Year - this is required for LoadSINQ
- First run number
- Optional: Last run number
- Optional: Number of files to merge
If only the first run number is given, the algorithm will only process that one file. If the last run number is also provided, the algorithm will process all files in the range, not merging anything. If the last parameter is given, it will merge the corresponding number of files together. Examples:
- 32322: Only 32322 is processed
- 32322, 32328: Files 32322 through 32328 are processed
- 32322, 32327, 2: Files 32322 & 32323 are merged, 32324 & 23325, 32326 & 32327 are merged. The resulting workspaces will have the name of the last data file in the range.
This algorithm will be used in the POLDI workflow algorithm.
Change History
comment:2 Changed 6 years ago by Michael Wedel
Refs #11285. Small change to PoldiMerge to get rid of temp workspace
Changeset: dfd822dc15cdcc09112e4ef9f199de0ad7bf71ed
comment:3 Changed 6 years ago by Michael Wedel
Refs #11285. Adding behavior to PoldiLoadRuns
Changeset: 8ce933037fb9c31ce94b7c78f09e6f54fa7ae9bf
comment:4 Changed 6 years ago by Michael Wedel
Refs #11285. Change logic of algorithm slightly
Changeset: 63038338190bac85dc2af20fb96e2a7a59ecd00d
comment:5 Changed 6 years ago by Michael Wedel
Refs #11285. Fixing year problem.
Changeset: b88353cea41fbc41cfd9749459d34c2ba2088fca
comment:6 Changed 6 years ago by Michael Wedel
Refs #11285. Added SystemTest
Changeset: ca268e2133193d831a00e2efd3a5f9b67cc2f800
comment:7 Changed 6 years ago by Michael Wedel
Refs #11285. More systemtests, added option to disable merge checks
Changeset: f47f5efa08a794b1787990b0ad4f247e2c784d38
comment:8 Changed 6 years ago by Michael Wedel
Refs #11285. Removing unit test
Discussed with Martyn, it's better to cover the different code paths in a system test instead of testing the private methods of the algorithm.
Changeset: 8faa31ab829fc0a8d1a9e161cd4d4d977a4c3f0d
comment:9 Changed 6 years ago by Michael Wedel
Refs #11285. Added documentation, usage examples
Changeset: 146db056ae880670b0a4eeef25c8d669677438a6
comment:10 Changed 6 years ago by Michael Wedel
- Status changed from inprogress to verify
- Resolution set to fixed
This is being verified as pull request #373.
comment:11 Changed 6 years ago by Michael Wedel
Jenkins retest this please
comment:12 Changed 6 years ago by Michael Wedel
Refs #11285. Forgot to remove unit test from CMakeLists.txt
Changeset: 1cccc3099d41530ed2e1fd32c89f999b8b387f98
comment:13 Changed 6 years ago by Michael Wedel
Jenkins retest this please
comment:14 Changed 6 years ago by Michael Wedel
Wrong ticket number in commit from other branch.
comment:15 Changed 6 years ago by Michael Wedel
The fail on windows was unrelated, but it's important to see that the doctests work also on that platform.
comment:16 Changed 6 years ago by Federico Montesino Pouzols
Jenkins, retest this please
comment:17 Changed 6 years ago by Michael Wedel
Refs #11285. Fixing some pylint problems
Changeset: 598e8aa850b20e4fd148ea77f45168809052ba9a
comment:18 Changed 6 years ago by Michael Wedel
Jenkins retest this please
comment:19 Changed 6 years ago by Federico Montesino Pouzols
- Status changed from verify to verifying
- Tester set to Federico Montesino Pouzols
comment:20 Changed 6 years ago by Federico Montesino Pouzols
All looks good and this new algorithm will be very convenient. All tests passed on the CI servers and also locally. The documentation reads well and It comes with fairly exhaustive tests and doc tests.
The only thing that occurred to me, which is not really specific to this ticket, is that maybe in PoldiLoadRuns-v1.rst and similar doc tests that create several workspaces you could use testcleanup sections to do a DeleteWorkspace on a few or all of them. Otherwise all the workspaces remain in the ADS. As the doc tests are run sequentially, some of the subsequent tests can be confused if a name conflict arises at some point. I guess in this case the chances are very low, as the workspace names include the run numbers. Last time I run the doc tests interactively I could see lots of workspaces being accumulated throughout the doc tests, using all sorts of funny names, and we could run into trouble at some point with this. On the other hand, that can be seen as a kind of convenient stress testing!
comment:21 Changed 6 years ago by Federico Montesino Pouzols
- Status changed from verifying to closed
Merge pull request #373 from mantidproject/11285_create_multiple_poldi_data_loader
Create algorithm that loads, checks and merges multiple POLDI data files
Full changeset: ab5bda8a6b3e7777e5c4fad280adcccd91eee9a2
comment:22 Changed 6 years ago by Michael Wedel
Yes that is actually something I have not considered so far. I just created a ticket to deal with it for all POLDI documentation (#11364(http://trac.mantidproject.org/mantid/ticket/11364)).
comment:23 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 12124
Refs #11285. First draft of algorithm, unit test
Changeset: 2fc439a0f2d839356460c3120ad9f818604c2591