Ticket #9369 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

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:

https://github.com/mantidproject/documents/blob/master/Design/Nested%20History%20Detailed%20Design%20Document.docx

Attachments

test_hist.py (391 bytes) - added by Samuel Jackson 6 years ago.
Testing script

Change History

comment:1 Changed 6 years ago by Owen Arnold

  • Status changed from new to assigned

comment:2 Changed 6 years ago by Samuel Jackson

  • Blocked By 9370 added

comment:3 Changed 6 years ago by Samuel Jackson

  • Blocked By 8922 added

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.

Changed 6 years ago by Samuel Jackson

Testing script

comment:39 Changed 6 years ago by Samuel Jackson

  • Blocked By 9370 removed

comment:40 Changed 6 years ago by Samuel Jackson

  • Blocking 9370 added

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:40 Changed 6 years ago by Samuel Jackson

  • Blocking 9370 added
  • Blocked By 9370 removed

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:56 Changed 6 years ago by Samuel Jackson

  • Blocking 9548 added

comment:57 Changed 6 years ago by Martyn Gigg

  • Status changed from verify to verifying

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

Note: See TracTickets for help on using tickets.