New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Modify mantid FilterEvents algorithm #35380

Merged

AndreiSavici merged 121 commits into main from ewm_737_738_739

May 5, 2023

Member

jmborr commented Mar 20, 2023 •

edited by bolotovskyr

Loading

References
187: [Enabler] Modify mantid FilterEvents algorithm #968

Description of work.

TODO's to resolve before merging the PR

FilterEvents::examineAndSortEventWS() sorts all EventList's in the input workspace but this seems unnecessary because TimeSplitter.splitEventList() sorts each EventList
Clone the input log properties into the output workspace. The prototype workspace prototype_ws should contain the logs of the input workspace, minus the ExcludeSpecifiedLogs. Then the cloning step optws = prototype_ws->clone() should also clone the logs seamlessly.

To test:

Refs #34794.

Reviewer

Please comment on the following (full description):

Code Review

Is the code of an acceptable quality?
Does the code conform to the coding standards?
Are the unit tests small and test the class in isolation?
If there is GUI work does it follow the GUI standards?
If there are changes in the release notes then do they describe the changes appropriately?
Are the release notes saved in a separate file, using Issue or PR number for file name and in the correct location?

Functional Tests

Do changes function as described? Add comments below that describe the tests performed?
Do the changes handle unexpected situations, e.g. bad input?
Has the relevant (user and developer) documentation been added/updated?

Does everything look good? Mark the review as Approve. A member of @mantidproject/gatekeepers will take care of it.

jmborr self-assigned this

This comment was marked as off-topic.

# to view

bolotovskyr requested review from bolotovskyr and removed request for bolotovskyr

March 24, 2023 21:59

bolotovskyr dismissed their stale review

March 24, 2023 22:03

Mistake

jmborr force-pushed the ewm_737_738_739 branch 2 times, most recently from 3f19b12 to c23540c Compare

March 29, 2023 15:01

jmborr force-pushed the ewm_737_738_739 branch from b9af436 to e972e48 Compare

April 6, 2023 16:47

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/src/FilterEvents.cpp Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/src/FilterEvents.cpp Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/src/FilterEvents.cpp Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/src/FilterEvents.cpp Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/src/FilterEvents.cpp Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/src/FilterEvents.cpp Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/src/FilterEvents.cpp Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/src/FilterEvents.cpp Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/test/FilterEventsTest.h Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/test/FilterEventsTest.h Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/test/FilterEventsTest.h Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/test/FilterEventsTest.h Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/test/FilterEventsTest.h Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/test/FilterEventsTest.h Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/test/FilterEventsTest.h Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/test/FilterEventsTest.h Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/test/FilterEventsTest.h Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/test/FilterEventsTest.h Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/test/FilterEventsTest.h Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Algorithms/test/FilterEventsTest.h Outdated Show resolved Hide resolved

peterfpeterson reviewed

View reviewed changes

Framework/Kernel/src/TimeSeriesProperty.cpp Outdated Show resolved Hide resolved

peterfpeterson requested changes

View reviewed changes

Member

peterfpeterson left a comment

Overall, this is a very impressive body of work. Lots of minor/cosmetic requests.

jmborr and others added 16 commits

May 3, 2023 07:33


          remove unused header

719ebf7

Signed-off-by: Jose Borreguero <borreguero@gmail.com>


          Expunge EventList::filterByTimeAtSample and EventList::filterByTimeAt…

06906f4

…SampleHelper

Signed-off-by: Jose Borreguero <borreguero@gmail.com>


          rearrange methods for easy jump between the methods and their helper …

dd41fd8

…methods

Signed-off-by: Jose Borreguero <borreguero@gmail.com>


          Fix incorrect comment and exception text.

072a04f


          copy the properties but filter them according to a TimeROI

892304f

Signed-off-by: Jose Borreguero <borreguero@gmail.com>


          include cppcheck supressions

70e1cbc

Signed-off-by: Jose Borreguero <borreguero@gmail.com>


          inserting replies to reviewer's comments

55eed62

Signed-off-by: Jose Borreguero <borreguero@gmail.com>


          Fix handling of single-value cache in ROI-based filtering methods.

2a91aee


          Fix unmatched cppcheck-suppress issues.

85516f6


          Implement LogManager::copyProperties method.

dbf67ac


          Update LogManager::setTimeROI

fa01365


          Updates to TimeROI-based filtering methods.

83f372b


          Empty-Commit

56f7de5


          New TimeSeriesProperty constructor. Use TimeROI intersection.

05e4b00


          Merge remote-tracking branch 'upstream/ewm1355_mean_math' into ewm_73…

f1fe680

…7_738_739


          Revert to main's copy of the reference file

dd55482

peterfpeterson force-pushed the ewm_737_738_739 branch from dd4a484 to dd55482 Compare

May 3, 2023 11:38

bolotovskyr and others added 2 commits

May 3, 2023 16:24


          Decouple setTimeROI and removeDataOutsideTimeROI.

b653843


          Make clients explicitly remove values outside of TimeROI

f762e23

bolotovskyr mentioned this pull request

Update LogManager::setTimeROI #35487

Closed

bolotovskyr and others added 6 commits

May 3, 2023 17:36


          Fix doxygen warning.

fef6dad


          Fix LogManager test.

97cead6


          Pass by reference

821f541


          Empty-Commit

c524362


          update golden data

e5f055e

Signed-off-by: Jose Borreguero <borreguero@gmail.com>


          Change names of timers

115947a

peterfpeterson approved these changes

View reviewed changes

AndreiSavici merged commit ed6c291 into main

AndreiSavici deleted the ewm_737_738_739 branch

May 5, 2023 13:24

rbauststfc mentioned this pull request

TimeSeriesProperty::splitByTimeVector sometimes returns unexpected results for missing log entries #35375

Closed

# for free to join this conversation on GitHub. Already have an account? # to comment

Labels

None yet