refnx / refnx

Neutron and X-ray reflectometry analysis in Python
https://refnx.readthedocs.io
BSD 3-Clause "New" or "Revised" License
37 stars 24 forks source link

Minor error in PolarizationEfficiency class matrix #618

Closed dcortie closed 2 years ago

dcortie commented 2 years ago

Hello,

There is a typo in the definition of the polarizer efficiency matrixes in the PolarisedEfficiency class (reduce.py).

self.flipper2_matrix = [ [one, z, one, z], [F2, (1 - F2), z, z], [z, z, one, z], [z, z, F2, (1 - F2)], ]

should be

self.flipper2_matrix = [ [one, z, z, z], [F2, (1 - F2), z, z], [z, z, one, z], [z, z, F2, (1 - F2)], ]

This will make it consistent with the original reference Review of Scientific Instruments 70, 4241 (1999) on Page 3. I found that using the incorrect matrix introduces a significant error (up to 30% over subtraction) in the NSF channel corrections when strong SF signals are also present.

Cheers, David

andyfaff commented 2 years ago

@dcortie thanks for finding + reporting the issue. Can you make a pull request for this?

  1. Fork refnx to your own account.
  2. Clone your refnx repository to your local computer
  3. Add the refnx/refnx repository as a remote: `git remote add refnx https://github.com/refnx/refnx.git
  4. Fetch any changes: git fetch refnx
  5. Make sure you're on the master branch: git checkout master
  6. Include all recent changes: git rebase refnx/master
  7. Make a feature branch: `git checkout -B myfirstPR
  8. Commit the changes: git commit -a -m'MAINT: fixed the bug in #618'
  9. Push the feature branch to your fork: `git push origin myfirstPR
  10. Go to the refnx/refnx repo and make a PR against the myfirstPR branch on your fork.

@ohcpaull, do you have feedback on this?

ohcpaull commented 2 years ago

Hi @andyfaff , yep @dcortie is completely right. I don't have any feedback other than being annoyed that I made such a silly mistake in the first place...