Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Extend LoadEventNexus For MultiPeriod Events #12963

Merged
merged 23 commits into from Jul 6, 2015

Conversation

OwenArnold
Copy link
Contributor

Issue #12776. Extend LoadEventNexus to handle multiperiod event nexus format files.

Create synthetic nperiods log entry. The nperiods entry is present in the file, but not in the logs section. Having this will make the logs given to LoadEventNexus contain the same nperiods entry we currently have on the histogram workspaces.
Before I convert this to a vector of workspaces, I notice that the same variable name WS is used both for local variables, and member variables. This is utterly stupid and bound to cause problems in the future, so i'm fixing the naming here first.
We want to know where we are explicitly accessing the held single member workspace, as this is going to change and will otherwise be the source of bugs.
Decorator now holds vector workspaces corresponding to periods.
Decorator initializes vector of workspaces once n-periods has been determined.
While maintaining backwards compatibility, ensure that any action done on our Decorated workspace is now done on the complete set of workspaces we have.
We do not allow loading of monitors in event mode for multiperiod event data.
@OwenArnold OwenArnold added High Priority An issue or pull request that if not addressed is severe enough to postponse a release. SANS Issues and pull requests related to SANS In Progress labels Jun 29, 2015
@OwenArnold OwenArnold self-assigned this Jun 29, 2015
@OwenArnold OwenArnold added this to the Release 3.5 milestone Jun 29, 2015
Fix warnings. Refactor DecoratorWorkspace into it's own physical unit. LoadEventNexus is already too big! A few tests added to that type in the process and forward declares used.
Some of the LET files have all zeros for the period number. We fix these
to be 1.
@@ -2776,7 +2820,7 @@ void LoadEventNexus::loadTimeOfFlightData(::NeXus::File &file,
* @param WS : pointer to the workspace
*/
void LoadEventNexus::loadSampleDataISIScompatibility(
::NeXus::File &file, Mantid::API::MatrixWorkspace_sptr WS) {
::NeXus::File &file, DecoratorWorkspace * const WS) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anyreason why WS can't be taken by non-const reference rather than pointer?

@martyngigg
Copy link
Member

This looks fine and the plots comparing .raw and rebinned event files look good.

martyngigg added a commit that referenced this pull request Jul 6, 2015
…loading

Extend LoadEventNexus For MultiPeriod Events
@martyngigg martyngigg merged commit 6d59e1d into master Jul 6, 2015
@martyngigg martyngigg deleted the 12776_multiperiod_event_loading branch July 6, 2015 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High Priority An issue or pull request that if not addressed is severe enough to postponse a release. SANS Issues and pull requests related to SANS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants