Ticket #9369 (closed: fixed)
Update algorithm history GUI for nested algorithm history
Reported by: | Samuel Jackson | Owned by: | Samuel Jackson |
---|---|---|---|
Priority: | major | Milestone: | Release 3.2 |
Component: | GUI | Keywords: | |
Cc: | Blocked By: | #8913, #8922 | |
Blocking: | #9370, #9548 | Tester: | Martyn Gigg |
Description
We need to upgrade the algorithm history GUI to support the nested history changes. This depends on ticket #8913 being completed first. The implemented GUI should be based on the mockup produced by Michael. For more information refer to the nested history design doc:
Attachments
Change History
comment:4 Changed 6 years ago by Samuel Jackson
- Status changed from assigned to inprogress
Refs #9369 Add HistoryView and HistoryItem classes.
Changeset: 9a8f3309cc174388829611bac2b874fe17370b42
comment:5 Changed 6 years ago by Samuel Jackson
Refs #9369 Add unit tests for HistoryItem and HistoryView
Changeset: 3e0c8c8741ab811ccb5851387ae9e5ee3d13a776
comment:6 Changed 6 years ago by Samuel Jackson
Refs #9369 Add new methods to HistoryView
We can now unroll/roll the entire history with one call.
Changeset: 3bd90ddaaa2e72cd715dd1831ffd14d06298260f
comment:7 Changed 6 years ago by Samuel Jackson
Refs #9369 Update unit test for HistoryView
This includes more verbose testing.
Changeset: d4cf4b542d887bcf00673d6b5e058afbd1d16dbe
comment:8 Changed 6 years ago by Samuel Jackson
Refs #9369 Add script builder class.
Builds a python script from a HistoryView.
Changeset: bfd51ca92dff2b072dae127755536238ab3c150a
comment:9 Changed 6 years ago by Samuel Jackson
Refs #9369 Improve ScriptBuilder output formatting.
Changeset: c9d108be1c167b94c6ba48d7f7622bfe373ba167
comment:10 Changed 6 years ago by Samuel Jackson
Refs #9369 Add unit tests for ScriptBuilder.
Changeset: dd5378ed927b6958405ddcf8d09006c1bb0890c0
comment:11 Changed 6 years ago by Samuel Jackson
Refs #9369 Add doxygen comments for classes.
Changeset: 372db944480256a87469ccde67306801842d8a35
comment:12 Changed 6 years ago by Samuel Jackson
Refs #9369 Update history GUI for nested history.
Changeset: e37e3f5157b7f0918534d71343b1204bccf4a3b4
comment:13 Changed 6 years ago by Samuel Jackson
Refs #9369 Only declare DPA test algs at test time.
Changeset: f09456ab0dc19fa50c30ff35ba9a11251b2d7561
comment:14 Changed 6 years ago by Samuel Jackson
Refs #9369 Swap to using pointers for PropertyHistory objects.
Changeset: f1c7e85b29703dd38fa0a62650c694f8888fb0d8
comment:15 Changed 6 years ago by Samuel Jackson
Refs #9369 Connect final child property name with parent output name.
Changeset: 56c6a59f2a7998c7f957274255067cf9605278d4
comment:16 Changed 6 years ago by Samuel Jackson
Refs #9369 Fix unit tests.
Changeset: e43d8b5e9b09e825a05425bedeec6092b300a799
comment:17 Changed 6 years ago by Samuel Jackson
Refs #9369 Tweak history interface size.
Changeset: 324b9d9969a3a1154bb24dd5d506f77361cfacec
comment:18 Changed 6 years ago by Samuel Jackson
Refs #9369 Refactor linking with child history.
Changeset: 3ed5fed769de5036cf5ef186bd6c0dd37ddfe102
comment:19 Changed 6 years ago by Samuel Jackson
Refs #9369 Get ScriptBuilder to write to file from GUI.
Changeset: 63ed55fe6f7cd41d9ece9a7857c596a70c785927
comment:20 Changed 6 years ago by Samuel Jackson
Refs #9369 Encapsulate temporary prop value creation and management.
Changeset: 1b1abae62d207d0b0f073c3ed06890cefd3215b6
comment:21 Changed 6 years ago by Samuel Jackson
Refs #9369 Fix comment spaces in script output.
Changeset: 6617dc9a934e699a59796b3c0b73567a5c459d99
comment:22 Changed 6 years ago by Samuel Jackson
Refs #9369 Let WorkspaceHistory create a view of itself.
Changeset: 0d3e458cbb097271cc240a41905117085a0a594a
comment:23 Changed 6 years ago by Samuel Jackson
Merge remote-tracking branch 'origin/master' into feature/9369_upgrade_history_gui
Refs #9369
Conflicts:
Code/Mantid/Framework/API/inc/MantidAPI/Algorithm.h Code/Mantid/Framework/API/inc/MantidAPI/AlgorithmHistory.h Code/Mantid/Framework/API/inc/MantidAPI/WorkspaceProperty.h Code/Mantid/Framework/API/src/AlgorithmHistory.cpp Code/Mantid/Framework/API/test/WorkspacePropertyTest.h
Changeset: c5b255f2c5774a73a0b4a27b05bcb415fd4e2720
comment:24 Changed 6 years ago by Samuel Jackson
Merge branch 'feature/9369_upgrade_history_gui' into develop
Refs #9369
Conflicts:
Code/Mantid/Framework/API/test/DataProcessorAlgorithmTest.h
Changeset: 3c93466635e46019073d5608c17fc3cf4e4dae4d
comment:25 Changed 6 years ago by Samuel Jackson
Refs #9369 Fix for OSX/RHEL6 extra qualification error.
Changeset: fd94b6d9d18f3024be892c300f93889e21e805f6
comment:26 Changed 6 years ago by Samuel Jackson
Refs #9369 Fix doxygen warnings.
Changeset: 92fc7b386501b488325d301cf204a61a62a0e844
comment:27 Changed 6 years ago by Samuel Jackson
Refs #9369 Fix for OSX not supporting crbegin/end
Changeset: 36840dec1b2c1cffd623522c450009bd0ddc764a
comment:28 Changed 6 years ago by Samuel Jackson
Refs #9369 Fix for OSX not supporting cbegin/cend on std::list
Changeset: 58e147f829ab4d34aa342aa355519d0d7902e8ab
comment:29 Changed 6 years ago by Samuel Jackson
Refs #9369 Fix rest of iterator issues.
Changeset: d8a771757ac944f9a16dcdcba1c8c89c8c06c3e2
comment:30 Changed 6 years ago by Samuel Jackson
Refs #9369 Missed one more iterator issue for OSX.
Changeset: 56d41b49e97ad909cf70a40d6cc75d43d1269fa8
comment:31 Changed 6 years ago by Samuel Jackson
Refs #9369 Missing include for OSX/RHEL6 build
Changeset: 74f33d450f52c05d1fc60c933d62bfee7206660b
comment:32 Changed 6 years ago by Samuel Jackson
Refs #9369 Add required summary method for dummy algorithms.
Changeset: 8c615dfab4ab584cb723794fab82b3484d7a47cc
comment:33 Changed 6 years ago by Samuel Jackson
Refs #9369 Missed const off of declaration of inherited member.
Changeset: 0c15a2ac77391cfebf22d0dc206b3b002825dbd2
comment:34 Changed 6 years ago by Samuel Jackson
Refs #9369 Missed one more dummy algorithm without summary.
Changeset: 4d26ee71a327448b77ca20ddc489b64322558ec5
comment:35 Changed 6 years ago by Samuel Jackson
Refs #9369 Add check for end of iterator.
Changeset: 14df27c6ed5d238c2372823813b5ef131939cc3e
comment:36 Changed 6 years ago by Martyn Gigg
There's a windows compiler warning that the clean build has flagged up: http://builds.mantidproject.org/job/develop_clean/label=win7-build/107/warnings30Result/
comment:37 Changed 6 years ago by Samuel Jackson
Refs #9369 Fix warning for MSbuild.
Changeset: 070973e337634895ab1f846a52dc3d795410522e
comment:38 Changed 6 years ago by Samuel Jackson
To Test
This ticket updates the history gui with the changes for nested history and also includes new classes to build a python script from the history which will eventually replace the code in GeneratePythonScript.
There's a python script attached to this ticket that runs CreateTransmissionWorkspaceAuto, run this script and open the history gui for the generated workspace. Check that you can view the history of all the child algorithms.
Now rename the workspace to be called something different (e.g. Test_orig). Check the unroll option on CreateTransmissionWorkspaceAuto (but NOT on CreateTransmissionWorkspace, this will not work yet due to #9517) and click script to clipboard. Paste the script into the script editor. Check the script looks reasonable. Run it. Now compare the generated workspace with the original workspace using CheckWorkspacesMatch. They should be identical.
Open the history on the second workspace. You should notice that the history is slightly different (no entry for CreateTransmissionWorkspaceAuto).
Finally, go back and view the history for the original workspace. Check to unroll everything, script it to clipboard (or file, to check both). Look at the script and check it matches what is in the history, but as mentioned this won't produce the same output yet.
comment:39 Changed 6 years ago by Samuel Jackson
- Status changed from inprogress to verify
- Resolution set to fixed
- Blocking 9370 removed
- Blocked By 9370 added
comment:41 Changed 6 years ago by Samuel Jackson
- Status changed from verify to reopened
- Resolution fixed deleted
comment:44 Changed 6 years ago by Samuel Jackson
- Status changed from reopened to inprogress
Refs #9369 Move linking method to outside fillHistory.
This needs to be outside fillHistory so that it can be overridden in concrete algorithms without have to reimplemented the linking.
Changeset: 6ee9547ea9527d8c60049315c96f56015323b98b
comment:45 Changed 6 years ago by Samuel Jackson
Merge branch 'feature/9369_upgrade_history_gui' into develop
Refs #9369
Conflicts:
Code/Mantid/Framework/API/test/DataProcessorAlgorithmTest.h
Changeset: 86fc1ed3ee81f04ccf7472935dcf2b38a2b3e91a
comment:46 Changed 6 years ago by Samuel Jackson
- Status changed from inprogress to verify
- Resolution set to fixed
comment:47 Changed 6 years ago by Martyn Gigg
- Status changed from verify to verifying
- Tester set to Martyn Gigg
comment:48 Changed 6 years ago by Martyn Gigg
After a glance at the code I can see that there are some headers that contain using declarations. These should not be in headers as they will pollute the namespace of anything that includes them. Ones I spotted:
- Code/Mantid/MantidPlot/src/Mantid/AlgorithmHistoryWindow.h
- Code/Mantid/Framework/API/inc/MantidAPI/AlgorithmHistory.h
Please could be removed and the appropriate prefixes applied in the headers.
I will keep testing the actual runtime behaviour. Feel free to commit while I still have the ticket for verifying.
comment:49 Changed 6 years ago by Martyn Gigg
- Status changed from verifying to reopened
- Resolution fixed deleted
I think I've found something that breaks this. The following script shows a use case from one of the system tests. The systemtests/Data directory will need to be in the path for the script to run
def runTest(): groupingFile=os.path.join(config.getString('defaultsave.directory'),'CNCS_powder_group.xml') parFile=os.path.join(config.getString('defaultsave.directory'),'CNCS_powder_group.par') nxspeFile=os.path.join(config.getString('defaultsave.directory'),'CNCS_powder_group.nxspe') vanFile=os.path.join(config.getString('defaultsave.directory'),'van.nx5') config['default.facility']="SNS" Load(Filename='CNCS_23936-23937',OutputWorkspace='sum') GenerateGroupingPowder(InputWorkspace="sum",AngleStep=0.5,GroupingFilename=groupingFile) Ei=mtd['sum'].getRun()['EnergyRequest'].firstValue() tib=SuggestTibCNCS(Ei) erange=str(-Ei)+','+str(0.01*Ei)+','+str(0.95*Ei) DgsReduction( SampleInputWorkspace="sum", OutputWorkspace="reduced", EnergyTransferRange="-0.2,0.05,2.2", GroupingFile=groupingFile, IncidentBeamNormalisation="ByCurrent", TimeIndepBackgroundSub=True, TibTofRangeStart=tib[0], TibTofRangeEnd=tib[1], DetectorVanadiumInputFile="CNCS_51936_event.nxs", UseBoundsForDetVan=True, DetVanIntRangeLow=52000.0, DetVanIntRangeHigh=53000.0, DetVanIntRangeUnits="TOF", SaveProcessedDetVan=True, SaveProcDetVanFilename=vanFile, ) ###################################################################################### runTest()
If you grab the history of the 'reduced' workspace, tell the DgsReduction algorithm to unroll and generate a script you get what looks like a correct script. If you try and execute it you get an error on the AddSampleLog line as the input workspace doesn't exist.
comment:50 Changed 6 years ago by Samuel Jackson
- Status changed from reopened to inprogress
Refs #9369 Remove using statements to stop namespace pollution.
Changeset: 246e128e66ad0ea2211ac786061df10d5827af3f
comment:51 Changed 6 years ago by Samuel Jackson
Refs #9369 Fix bug in algorithm history linking mechanism.
Changeset: b08efce1129b3e711763f2ec215308d908269939
comment:52 Changed 6 years ago by Samuel Jackson
Refs #9369 Remove setting a child output workspace name in DPAs.
This causes issues in history because some properties will use a temporary name and some will use the more descriptive name.
Changeset: 7b8d17232229a714749466b964311bc0b63c52e9
comment:53 Changed 6 years ago by Samuel Jackson
Refs #9369 Remove unused variables.
This should fix the CppCheck warnings on develop.
Changeset: bcf0e072cc4f1bb89838e4efd8b1ae713dceec53
comment:54 Changed 6 years ago by Samuel Jackson
- Status changed from inprogress to verify
- Resolution set to fixed
comment:55 Changed 6 years ago by Samuel Jackson
Script provided now appears to work. I've searched through the other data processor algorithms we already have in Mantid and removed any lines that cause this issue. I've also removed the using declarations from header files as requested.
comment:58 Changed 6 years ago by Martyn Gigg
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/feature/9369_upgrade_history_gui'
Conflicts:
Code/Mantid/Framework/API/test/DataProcessorAlgorithmTest.h
Full changeset: 08c2449d2a3c06987dd55c3ae851198a2876c2d4
comment:59 Changed 6 years ago by Samuel Jackson
Merge branch 'feature/9369_upgrade_history_gui' into develop
Refs #9369
Conflicts:
Code/Mantid/Framework/API/test/DataProcessorAlgorithmTest.h
Changeset: 86fc1ed3ee81f04ccf7472935dcf2b38a2b3e91a
comment:60 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 10212