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.

Sign up for GitHub

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

Jump to bottom

Indirect Diffraction - OSIRIS diffonly interface crash #18857

Merged
merged 3 commits into from
Feb 16, 2017
Merged

Indirect Diffraction - OSIRIS diffonly interface crash #18857

merged 3 commits into from
Feb 16, 2017

Conversation

louisemccann
Copy link
Contributor

@louisemccann louisemccann commented Feb 14, 2017 •
edited
Loading

The OSIRIS diffonly mode of the Indirect>Diffraction interface was crashing because it was trying to run the general IRISIndirectDiffractionReduction on top of the specialised OSIRISDiffractionReduction. It should only run the OSIRIS algorithm.

To test:
Input Files:
Diffraction.zip

Interfaces>Indirect>Diffraction
Instrument:OSIRIS
Input: 89813-89817
Calibration: In input files
Vanadium:89757-89761
Press Run

There should be 2 workspaces, one ending in _dRange and the the other in tof
There are copies of the input/vanadium files if no access to ISIS data archive

Fixes #18856.

Release notes have been updated

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? Is it well structured with small focussed classes/methods/functions?
  • Are there unit/system tests in place? Are the unit tests small and test the a class in isolation?
  • If there are changes in the release notes then do they describe the changes appropriately?
Functional Tests

  • Do changes function as described? Add comments below that describe the tests performed?

  • How do the changes handle unexpected situations, e.g. bad input?

  • Has the relevant documentation been added/updated?

  • Is user-facing documentation written in a user-friendly manner?

  • Has developer documentation been updated if required?

  • Does everything look good? Comment with the ship it emoji but don't merge. A member of @mantidproject/gatekeepers will take care of it.

@louisemccann louisemccann added Component: GUI Indirect/Inelastic Issues and pull requests related to indirect or inelastic Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) labels Feb 14, 2017
@louisemccann louisemccann added this to the Release 3.10 milestone Feb 14, 2017
@louisemccann louisemccann added the Patch Candidate Urgent issues that must be included in a patch following a release label Feb 15, 2017
@raquelalvarezbanos
Copy link
Contributor

Works as described: the interface didn't crash and two workspaces ending in _dRange and _tof were created in the ADS.

:shipit: when all tests pass.

@raquelalvarezbanos
Copy link
Contributor

@mantidproject/gatekeepers: Could this be merged? Sanghamitra is waiting for this fix.

@SimonHeybrock SimonHeybrock merged commit a3147bb into master Feb 16, 2017
@SimonHeybrock SimonHeybrock deleted the 18856_indirect_diff_osiris_crash branch February 16, 2017 09:27
@raquelalvarezbanos
Copy link
Contributor

Thanks @SimonHeybrock

martyngigg pushed a commit that referenced this pull request Feb 23, 2017
@louisemccann @martyngigg
Cherry pick pull request #18857
b55eca5 
Refs #18856 don't run general diffraction if OSIRIS diffonly

(cherry picked from commit abc0d75)

Refs #18856 spelling etc

(cherry picked from commit 7a233cf)

Add patch release note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@raquelalvarezbanos raquelalvarezbanos

Labels
Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) Indirect/Inelastic Issues and pull requests related to indirect or inelastic Patch Candidate Urgent issues that must be included in a patch following a release
Projects
None yet
Milestone
Release 3.9.1
Development

Successfully merging this pull request may close these issues.
4 participants
@louisemccann @raquelalvarezbanos @martyngigg @SimonHeybrock