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

Fixed muon fitting bugs #29672

Merged
merged 3 commits into from
Oct 8, 2020
Merged

Fixed muon fitting bugs #29672

merged 3 commits into from
Oct 8, 2020

Conversation

AnthonyLim23
Copy link
Contributor

A user has reported a couple of bugs in muon analysis. Check that you can cause the bugs on your dev build before testing the fix.

Bug 1:
Open muon analysis
Select the EMU instrument
Load 84447-9 (and repeat with 84447-9)
Go to fitting
Add dynamicKobyeTobye
Fix the field
Tick simultaneous fitting
Change from run to group/pair
Change the field values (all of them should be fixed) to have unique values (e.g. 1,2,3 ...)
Click the increment run button or load current run
CRASH!

Bug 2:
Open muon analysis
Load some data
Go to fitting
Add a function
Manually change a value
Errors have gone

After fix 2, the errors will still disappear, but will come back after doing a fit.

To test:

Fixes #29669.


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?
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.

@AnthonyLim23 AnthonyLim23 added Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) Patch Candidate Urgent issues that must be included in a patch following a release ISIS Team: Spectroscopy Issue and pull requests managed by the Spectroscopy subteam at ISIS Reported By User Issues that were found or highlighted by a user/scientist labels Oct 6, 2020
@Matt-Cumber Matt-Cumber self-assigned this Oct 6, 2020
Matt-Cumber
Matt-Cumber previously approved these changes Oct 6, 2020
Copy link
Contributor

@Matt-Cumber Matt-Cumber left a comment

Choose a reason for hiding this comment

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

Good job fixing both of these bugs 👍 The crash no longer appears and the errors re-appear after fitting.

Copy link
Contributor

@Matt-Cumber Matt-Cumber left a comment

Choose a reason for hiding this comment

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

All tests pass

@martyngigg martyngigg merged commit b5f20b7 into release-next Oct 8, 2020
@martyngigg martyngigg deleted the muonCrashes branch October 8, 2020 14:40
@martyngigg martyngigg added this to the Release 5.1.1 milestone Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) ISIS Team: Spectroscopy Issue and pull requests managed by the Spectroscopy subteam at ISIS Patch Candidate Urgent issues that must be included in a patch following a release Reported By User Issues that were found or highlighted by a user/scientist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants