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

Allow user to select frequency units in RRFMuon #201

Merged
merged 8 commits into from Feb 12, 2015

Conversation

raquelalvarezbanos
Copy link
Contributor

Fixes trac issue #11028

To test:

  • Check code changes (.cpp, .h, unit test and documentation)
  • Try user example and play with the different options

@raquelalvarezbanos
Copy link
Contributor Author

Jenkins, retest this please

@peterfpeterson peterfpeterson added this to the Release 3.4 milestone Feb 11, 2015
@eXeC64 eXeC64 self-assigned this Feb 12, 2015
@eXeC64
Copy link
Contributor

eXeC64 commented Feb 12, 2015

Looks good, there's just a couple of things I don't understand.

  • Reference 1 The TS_ASSERT_DELTA makes sense, but if we want to be sure they're the same value, ignoring precision errors, why have TS_ASSERT_DIFFERS? Wouldn't them having the same exact value just be a happy coincidence, not an error?
  • Reference 2 This may be my ignorance showing (I'm just a lowly computer scientist after all 😉), but a comment explaining what exactly the 135.538.. value is and where it comes from would probably be useful for future reference.

@raquelalvarezbanos
Copy link
Contributor Author

You are right about Reference 2, I should have added a comment. That number corresponds to the muon gyromagnetic ratio (in MHz/T), I will update the code accordingly.
Regarding Reference 1, what I want to check is that both results are similar, but they should not be exactly the same. If they were, something would be wrong. I will also add some explanation.
Thanks for your comments :)

eXeC64 added a commit that referenced this pull request Feb 12, 2015
…_in_RRFMuon

Allow user to select frequency units in RRFMuon
@eXeC64 eXeC64 merged commit 307e638 into master Feb 12, 2015
@eXeC64 eXeC64 deleted the 11028_add_frequency_widget_in_RRFMuon branch February 12, 2015 14:12
mkoennecke added a commit to mkoennecke/mantid that referenced this pull request Feb 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants