Ticket #8959 (closed: fixed)
SaveReflCustom save Algorithm
Reported by: | Keith Brown | Owned by: | Lottie Greenwood |
---|---|---|---|
Priority: | major | Milestone: | Release 3.4 |
Component: | Reflectometry | Keywords: | NewStarter |
Cc: | Blocked By: | ||
Blocking: | #8963, #10932 | Tester: | Federico M Pouzols |
Description (last modified by Owen Arnold) (diff)
Reflectometry Custom is a file format the ISIS reflectometry gui can save to, but at present it's written in python.
Write a c++ algorithm for it.
Change History
comment:3 Changed 6 years ago by Keith Brown
- Owner changed from Keith Brown to Owen Arnold
not going to get this done before i leave, giving to owen
comment:5 Changed 6 years ago by Anders Markvardsen
- Owner changed from Owen Arnold to Lottie Greenwood
- Keywords NewStarter added
Do not hesitate to ask Owen for further information
comment:6 Changed 6 years ago by Owen Arnold
Please write the new algorithm, and then replace the functionality in the GUI code to use your new algorithm instead.
1) Ensure that the new code is unit tested.
2) Check that the functionality the the reflectometry gui has not changed by running the old code and rechecking against your new implementation once you have swapped that in. The ascii files generated should be identical.
comment:8 Changed 6 years ago by Lottie Greenwood
Commit 01 Refs #8959 checkpointing
Changeset: 502dc11510f65f66e3d32414fcbb81338eb9fe80
comment:9 Changed 6 years ago by Lottie Greenwood
Commit 2 Refs #8959 checkpoint
Changeset: 826443f68fe774a3b84c082f17c3161b60f49dfc
comment:10 Changed 6 years ago by Lottie Greenwood
Commit 3 Refs #8959 added new saveAlgorithm with custom header detail
Changeset: 1696e98190943c095a90a61cc9b535cc1a184910
comment:11 Changed 6 years ago by Lottie Greenwood
Commit 4 Refs #8959 adjusted AsciiPointBase class data method
Changeset: 7f2ad13cd973f36a8e1ad9afb0492cedcc138845
comment:12 Changed 6 years ago by Lottie Greenwood
Commit 5 Refs #8959 finished working ReflCustomAscii class
Changeset: 0a7898383917263d2479cf9509c5a1eae6ff7618
comment:13 Changed 6 years ago by Lottie Greenwood
Commit 6 Refs #8959 wrote unittest
Changeset: c2e020679f78fc26f7a5135abd275c77d4796acd
comment:14 Changed 6 years ago by Lottie Greenwood
Refs #8959 Removed unnec variable
Changeset: bc80043a3e316f432a508069176518d335f121ce
comment:15 Changed 6 years ago by Lottie Greenwood
Refs #8959 remove unnec code
Changeset: 6e60f60a04b4ff65cd666bddbd184d3b857ed788
comment:16 Changed 6 years ago by Lottie Greenwood
Refs #8959 Finished unit test
Changeset: 2f0762c6af0141b886ce550bd976166dd84074f8
comment:17 Changed 6 years ago by Lottie Greenwood
Refs #8959 Fixed for cpp Check errors
Changeset: 89f5cba15b5b2977af5608d1075b73b36c377349
comment:18 Changed 6 years ago by Lottie Greenwood
Refs #8959 Added in subtitle option, tested
Changeset: c05622b4d084d06588975b04a8bf1ccf03d8f8b3
comment:19 Changed 6 years ago by Lottie Greenwood
Refs #8959 fix merge conflicts
Changeset: de1b4b7b1e351a151cc2298572bc04ca6eca7f1f
comment:20 Changed 6 years ago by Lottie Greenwood
Refs #8959 Fix for tests
Changeset: a8ef72249e314369f6b653daf71deed357197b15
comment:21 Changed 6 years ago by Lottie Greenwood
- Milestone changed from Release 3.3 to Release 3.4
comment:22 Changed 6 years ago by Lottie Greenwood
Refs #8959 passed unit tests
Changeset: 45d2de31dcb8abc4185fcf02c0aceb0637b2153a
comment:23 Changed 6 years ago by Lottie Greenwood
To test:
- Interfaces > Reflectometry > ISIS Reflectometry > Enter Runs = 13460 > Process
- Run SaveReflCustom Ascii, check returns 3 columns of x,y, e if WriteDeltaQ unchecked, and 4 columns if it is
- From the ISIS Reflectometry interface mentioned above, use the original SaveReflCustom Algorithm and check the results do not differ from the new version
comment:24 Changed 6 years ago by Lottie Greenwood
- Status changed from inprogress to verify
- Resolution set to fixed
comment:25 Changed 6 years ago by Federico M Pouzols
- Status changed from verify to verifying
- Tester set to Federico M Pouzols
comment:26 Changed 6 years ago by Federico M Pouzols
I'm having a look at the code and it all looks good to me. Just one q: have you added an .rst documentation file for the algorithm? Like in https://raw.githubusercontent.com/mantidproject/mantid/master/Code/Mantid/docs/source/algorithms/LoadNexusProcessed-v1.rst for example, which ends up in http://docs.mantidproject.org/nightly/algorithms/LoadNexusProcessed-v1.html.
Or, is that included in another ticket?
comment:27 Changed 6 years ago by Federico M Pouzols
Oh, also, the branch is not found when I try a git start test .... I think that you need to do a checkbuild now with the new develop branch.
comment:28 Changed 6 years ago by Federico M Pouzols
I also noticed that the files are not exactly in clang format even though they go inside Framework/. When you do the checkbuild it would be a good idea to run clang-format on them. Well, I think that since some time in late December there's a git hook that will do it anyway.
comment:29 Changed 6 years ago by Martyn Gigg
- Status changed from verifying to reopened
- Resolution fixed deleted
There are also some compiler warnings on the clean build - http://builds.mantidproject.org/job/develop_clean/610/warnings20Result/
comment:30 Changed 6 years ago by Lottie Greenwood
- Status changed from reopened to inprogress
Refs #8959 clang formatted and unused parameters removed
Changeset: 6d2643fa6fda1541ef2748f5f4425aa50305849e
comment:31 Changed 6 years ago by Lottie Greenwood
Refs #8959 SaveReflTest fixed
Changeset: 5e3e7ca8ca75bebe439e0b20add32df5b9bc6d8b
comment:32 Changed 6 years ago by Lottie Greenwood
Refs #8959 SaveReflTest fixed for cppWarnings
Changeset: b90b7e7e1ed6fabb01131c31e42ae1dca7967ab5
comment:33 Changed 6 years ago by Lottie Greenwood
Refs #8959 SaveReflTest fixed for cppWarnings
Changeset: 3f66b092766897c3c5ec8912ba4ec3bf04372d25
comment:34 Changed 6 years ago by Lottie Greenwood
- Status changed from inprogress to verify
- Resolution set to fixed
comment:35 Changed 6 years ago by Lottie Greenwood
Documentation req in separate ticket (#10932)
To test see comment:23
comment:36 Changed 6 years ago by Martyn Gigg
Just as an FYI - You can delete branches from the GitHub interface here. If you type the ticket number in to the search box then you can delete the one that you don't want anymore.
comment:38 Changed 6 years ago by Federico M Pouzols
Just a detail: remember to update 2015 in the copyright line when you edit these files later on.
All looks good to me. Tests pass, and I think all the warnings from the build servers were sorted out.
I'd say this ticket is ready, but as there's been ups and downs in the build servers, I'll wait until tomorrow before merging it into master, just in case there would be any minor issue hidden in between the builds.
comment:39 Changed 6 years ago by Federico M Pouzols
Aha, the fedora and ubuntu build_clean are still showing warnings about SaveReflCustomAsciiTest.h, lines 224, and 230: two method parameters set but not used: propertiesLogs, and createLogs.
comment:40 Changed 6 years ago by Lottie Greenwood
Refs #8959 fix cpp warnings
Changeset: eda674162a3d09c7d1e92fcbb5610c8dd8e4da97
comment:41 Changed 6 years ago by Lottie Greenwood
Refs #8959 fix cpp warnings
Changeset: 0b40dd5b340c6bcbac82439e5ec79e069c17ea5a
comment:43 Changed 6 years ago by Federico Montesino Pouzols
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/feature/8959_SaveReflCustomAlgoritm'
Full changeset: 0645e3788817bc0f1d5631f24706ee654e56e363
comment:44 Changed 6 years ago by Federico M Pouzols
It works well, the algorithm saves the same values as the old python implementation (with 3 or 4 columns) in tab delimited text files.
This has been building happily and I also checked that all tests pass locally.
comment:45 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 9802
Bulk move of tickets out of triage (new) to assigned at the introduction of the triage state