Ticket #8913 (closed: fixed)
History depth / extracting scripts
Reported by: | Nick Draper | Owned by: | Samuel Jackson |
---|---|---|---|
Priority: | major | Milestone: | Release 3.2 |
Component: | Framework | Keywords: | SSC,2014,All |
Cc: | Blocked By: | ||
Blocking: | #7075, #8922, #8928, #9369, #9370, #9404, #9409 | Tester: | Anders Markvardsen |
Description (last modified by Nick Draper) (diff)
Design doc is https://github.com/mantidproject/documents/blob/master/Design/Nested%20History%20Detailed%20Design%20Document.docx
Notes from estimation
"Need to finish inital design with more concentration on python script extraction. Requires changing history storage to a tree. Extract History as python algorithm"
Attachments
Change History
comment:3 Changed 7 years ago by Nick Draper
- Status changed from new to assigned
Bulk move of tickets out of triage (new) to assigned at the introduction of the triage state
comment:4 Changed 7 years ago by Russell Taylor
- Blocking 7075 added
(In #7075) Take another look at this after the impending history refactoring.
comment:8 Changed 6 years ago by Samuel Jackson
- Status changed from assigned to inprogress
Refs #8913 Add additional getter/setters to AlgorithmHistory.
Changeset: add7c42c454ae7bbe1cab4d5720fb77fe597f44e
comment:9 Changed 6 years ago by Samuel Jackson
Refs #8913 Add new methods to Algorithm class for nested history.
Changeset: 96c50a5f91cd076aac6fb059e06d4bd279780e05
comment:10 Changed 6 years ago by Samuel Jackson
Refs #8913 Modify Algorithm and DataProcessorAlgorithm
These classes should now capture the history of child algorithms.
Changeset: eec355f39eb1baac78008c321d49422e340c393a
comment:11 Changed 6 years ago by Samuel Jackson
Refs #8913 Give workspace properties a temporary name if required.
Changeset: d6312ec79e7385877941ce8fa002c136cf13570a
comment:12 Changed 6 years ago by Samuel Jackson
Refs #8913 Add nested history test in AlgorithmHistory.
Changeset: 53ba3f0917d693c824fa1f1a872ab19afb51d00f
comment:13 Changed 6 years ago by Samuel Jackson
Refs #8913 Correct implementation logic between Algorithm and DPA.
Changeset: 16ef51b96fc1be3731fc9486b3c1bcc526dbee0f
comment:14 Changed 6 years ago by Samuel Jackson
Refs #8913 Add a couple of additional methods for convenience.
These are consistent with the functionality that is available in WorkspaceHistory.
Changeset: d8481c4dd4616d4444d0e48c8bf8966b3495bd68
comment:15 Changed 6 years ago by Samuel Jackson
Refs #8913 Fix bug(s) with copying algorithm history objects.
Changeset: 57d00a074cc8be43b2c74597dd3c7d28f650cd59
comment:16 Changed 6 years ago by Samuel Jackson
Refs #8913 Add the ability to turn off algorithm recording.
This changeset allows us to use enableHistoryRecordingForChild() to turn off recording the history for a DataProcessorAlgorithm. This will be useful when we have large algorithm histories.
Changeset: 98b5631ae0e450c96cce4fc303307cb4d42ee36b
comment:17 Changed 6 years ago by Samuel Jackson
Refs #8913 Add unit tests for nested history in DataProcessorAlgorithm
Changeset: 6d43b7c247d6c26205b611eb70aa3d0c9b3847f0
comment:18 Changed 6 years ago by Samuel Jackson
Refs #8913 Set exec count after algorithm is executed.
Otherwise LoadProcessNexus modifies the execution counter when it loads histories from file and the parent algorithm gets the incorrect exec count.
As you should never need to set the exec count except in Algorithm, I've made the method private and made Algorithm a friend class.
Changeset: 93d0174d8b02a186ba1f3d13c4a39b29c1d1b248
comment:19 Changed 6 years ago by Samuel Jackson
Refs #8913 Modify PropertyHistory constructor to take property pointer
This also gives WorkspaceProperty a temp value if it it doesn't have one.
Changeset: 3c546d72862f56869943c41e6039893359994a32
comment:20 Changed 6 years ago by Samuel Jackson
Refs #8913 Add method to create child algorithms from history.
Changeset: 7be99b4ee79fe9b601abfadb8fd248b60f8be97c
comment:21 Changed 6 years ago by Samuel Jackson
Refs #8913 Export Workspace/Algorithm/Property histories to python
Changeset: 2c4811eb11769454b665adbeb948a551d9de82a7
comment:22 Changed 6 years ago by Samuel Jackson
Refs #8913 Fix failing unit python tests.
Changeset: 47cc22ebfc77739041ce0e8a74eb9bc3e8ed3074
comment:23 Changed 6 years ago by Samuel Jackson
Refs #8913 Add python API unit tests.
Changeset: 07c31484e5c44b54e3ceb8b7915c2fafe223baba
comment:24 Changed 6 years ago by Samuel Jackson
Refs #8913 Pass history recording parameter as arg to createChildAlg.
Changeset: 897f6072894627d1d2f5cec653378609de366a7e
comment:25 Changed 6 years ago by Samuel Jackson
Refs #8913 Add unit test for disabling history in python
Changeset: 29ec004e08f25a09af31031aec06bc309c86abf4
comment:26 Changed 6 years ago by Samuel Jackson
Refs #8913 Remove redundant code from python simpleapi.
Changeset: 576c19ee4af3404499fa6cc9ba08fd7b3a0eebfe
comment:27 Changed 6 years ago by Samuel Jackson
Refs #8913 Export createChildAlgorithm in DPA.
Changeset: ad7e6127e201df7f75188cc4c9f7194dacb19870
comment:28 Changed 6 years ago by Samuel Jackson
Refs #8913 Fix broken unit tests.
Changeset: 0efcd8774fa5e3b6f9e1013e921a9aa30984e98e
comment:29 Changed 6 years ago by Samuel Jackson
Refs #8913 Update unit test for AlgorithmHistory
Changeset: f5e88093c8c16b1625e83efa98665085c797763c
comment:30 Changed 6 years ago by Samuel Jackson
Refs #8913 Refactor to use existing method.
No need for a new method as there is already a workable option.
Changeset: 866666526dacd75ca0da74bdd429286602a7af2e
comment:31 Changed 6 years ago by Samuel Jackson
Refs #8913 Fix doxygen warnings.
Changeset: 803df7055f50bd0c0b96f88cde9922c940a2433a
comment:32 Changed 6 years ago by Samuel Jackson
Refs #8913 Fix for RHEL6 unit test.
The assertGreater function wasn't available in python 2.6
Changeset: fd6efdcaf2f3c6f5ef530c8a3f5c2747d2ca491b
comment:34 follow-up: ↓ 38 Changed 6 years ago by Russell Taylor
Big change in the CreateWorkspaceTestPerformance.testBigWorkspace performance test that needs to be understood and remedied.
Doing so might not be too hard - this appears in the test source code:
// The AlgorithmHistory operations take an age - this disables them creator.setChild(true);
Presumably this is no longer true so another way needs to be found because we don't want one test taking 200s. It also would be nice to have them not take an age - they do so because it's putting every element in a very large ArrayProperty into the history. Perhaps we should not do that by default.
There are other tests that show a small degradation at this point, but nothing like the CreateWorkspace one.
comment:35 Changed 6 years ago by Samuel Jackson
Refs #8913 Stop recording history for CreateWorkspace performance test
Changeset: c5b39d5aeffdc86fc721b0ba6193f8450210b81c
comment:37 Changed 6 years ago by Samuel Jackson
Refs #8913 Move history creation to be conditional.
We should only create a history record if we're tracking it as we only discard it later.
Changeset: a44f2bcfd8f7bea82505a29c14cfeb8efba1edd5
comment:38 in reply to: ↑ 34 Changed 6 years ago by Samuel Jackson
Replying to Russell Taylor:
I've turned off history recording for CreateWorkspaceTestPerformance for now as suggested. I also realised that I was creating history records and simply discarding them after I moved where history objects get created. Fixing this appears to have knocked time back off of the other performance tests too.
However the issue with large vectors in PropertyHistory still needs to be investigated. I've made ticket #9409 to deal with this as those changes can be done independently of this ticket.
Big change in the CreateWorkspaceTestPerformance.testBigWorkspace performance test that needs to be understood and remedied.
Doing so might not be too hard - this appears in the test source code:
// The AlgorithmHistory operations take an age - this disables them creator.setChild(true);Presumably this is no longer true so another way needs to be found because we don't want one test taking 200s. It also would be nice to have them not take an age - they do so because it's putting every element in a very large ArrayProperty into the history. Perhaps we should not do that by default.
There are other tests that show a small degradation at this point, but nothing like the CreateWorkspace one.
comment:39 Changed 6 years ago by Samuel Jackson
Refs #8913 Start time should be after history creation.
A large history record could noticeably increase the execution time of the concrete algorithm otherwise.
Changeset: 087250da56fc5fffd696ddf7079ff037c0b49ad3
comment:40 Changed 6 years ago by Russell Taylor
The Jenkins valgrind job is complaining about an invalid read (i.e. a read after deletion) in the AlgorithmHistory code. Details here.
comment:41 Changed 6 years ago by Samuel Jackson
Refs #8913 Check that workspaces without a name get a temp name.
Changeset: c3af2f5216dab6e26bec5b96895e1dea4c238cac
comment:42 Changed 6 years ago by Samuel Jackson
Refs #8913 Don't assign history to itself.
Check if this is what is causing memcheck to fail.
Changeset: abb376f694e63fbbbe5478db68e8cc1870c7fbc1
comment:43 Changed 6 years ago by Samuel Jackson
Refs #8913 Swap AlgorithmHistory to use shared pointers
Changeset: 5260df9d21182af9d54f580478cf95a646a14b6b
comment:44 Changed 6 years ago by Samuel Jackson
Refs #8913 Add WorkspaceHistory performance test.
Changeset: 296f53f1a96a86da1e73bbba33df71970c5e2f6f
comment:45 Changed 6 years ago by Samuel Jackson
Refs #8913 Add remove const return policy for RHEL6.
Changeset: 55b68e44c5d73ef702def639ccb6ef8701641f4e
comment:46 Changed 6 years ago by Samuel Jackson
Refs #8913 Missed namespace declaration.
Changeset: bf57e31d0190015adce9b28453c341c6d59b1c87
comment:47 Changed 6 years ago by Samuel Jackson
Refs #8913 Remove const reference export.
Changeset: 2a5e9133459cf2293ad6ab7425f01f752602ee33
comment:48 Changed 6 years ago by Samuel Jackson
Refs #8913 Fix turning off algorithm history.
Changeset: 10d21cfb8ad6daa49ca205ad7aa8d98492bc97a5
comment:49 Changed 6 years ago by Samuel Jackson
Refs #8913 Fix Doxygen warnings for missing parameter.
Changeset: 7cea29a8b61519a97877e81d865276bff2356ddf
comment:50 Changed 6 years ago by Samuel Jackson
This ticket just contains the changes to the internal algorithm history structure. File IO, GUI and scripting changes are for other tickets that can be done in parallel once this ticket is merged.
To Test
First the obvious stuff:
- Check the unit tests are passing
- Check the system tests are passing
- Check that valgrind and static analysis jobs are not flagging any issues
- Check the performance tests are reasonable
The changes are largely hidden at the moment, but you can still test that nested history is working using the Python API. I've provided an example test script as a demo of basic operation. Check that:
- Regular algorithms do not record their children unless explicitly told to do so
- Algorithms inheriting from DataProcessorAlgorithm automatically record the history of child algorithms.
- Nested histories appear in the correct order that they were executed at each level in the tree (oldest first).
- Properties and meta information are recorded correctly.
- The python API functions work as you would expect.
- Code review.
comment:52 Changed 6 years ago by Samuel Jackson
- Status changed from inprogress to verify
- Resolution set to fixed
comment:53 Changed 6 years ago by Anders Markvardsen
- Status changed from verify to verifying
- Tester set to Anders Markvardsen
comment:54 Changed 6 years ago by Anders Markvardsen
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/feature/8913_nested_algorithm_history'
Full changeset: c218dee08ee7e3ebda8e43434127db88e5e01823
comment:55 Changed 6 years ago by Anders Markvardsen
Worked through various different scenarios of attached script and found worked as expected. Performance test increase has been dealt with etc
comment:57 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 9756