Unify ASAD and Stokes method polarization code#620
Draft
jdbuhler wants to merge 23 commits into
Draft
Conversation
code as closely as possible
* test whether we can merge the mu100 calculations * turn the actual Stokes fitting code into a fit() function analogous to that in the ASAD method * convert gratuitous printing to logging * simplify some code in the ASAD method after comparing it to the Stokes method
* track down sqrt warning in test case; this seems to be an error in implementing error estimates in Stokes method?
* energy trimming applies to all properties of events, not just the Phi/PsiChi (in particular, to time) * enable plotting in tests to exercise that code
* add some documentation
* add explicit plotting code for Stokes parameters * tweak tutorials to match changes in API * drop some parameter printing from the Stokes tutorial where the data is not readily available
sensible results * reformat existing no-bg Stokes test to match other tests
Documentation build overview
14 files changed ·
|
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
…nally by response-based simulated ASAD generation
spacecraft-frame fit convention (and fix a typo it uncovered) * convert base class members exported to subclasses into properties
…hanged explicitly in latest cosipy), and document strategies used to obtain scattering angles
small adjustments to uncertainty estimates used only for plotting. I think this now matches the equations in the paper, given that the underlying Stokes parameters are 2x larger in the code vs the paper.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The ASAD and Stokes method code to determine polarization angle and fraction share a great deal of functionality, though it is implemented differently in each method. This patch extracts the common functionality from each method into a base class, PolarizationFitting, and derives PolarizationASAD and PolarizationStokes from that. Besides avoiding a great deal of code duplication, this change lets us use the fast ASAD calculations from the ASAD method in the Stokes method as well, which reduces the latter's overall running time to a fraction of a second.
The common code is mainly in computing scattering angles from data/background and in computing ASADs as input to the mu100 and mdp99 calculations. The actual fitting code is, of course, quite different and is implemented separately in each subclass. The other key differences are that the Stokes code supports calculation in the absence of background data, which the ASAD code does not, and that the ASAD method supports binned input data, which the Stokes code does not. Originally, the Stokes code used a different implementation of the mu100 calculation that produced slightly different answers, but after consulting with @eneights and @nmik , I used the ASAD code's implementation for both methods. (This can easily be changed if desired.)
I also reviewed the correspondence between the Stokes fitting code and the paper (Kislat et al. 2015) that it draws from. I think there is a missing factor of mu and an I vs I-1 issue in the code that implements the uncertainty calculations according to Equations 28 and 29. I marked the modified code "FIXED", but it should be reviewed.
The ASAD and Stokes notebooks have been updated to the API of the new implementation, which involved renaming a couple of properties and extracting some plotting code for Stokes into its own function. The notebook versions on this branch show the output from each method under the unified code.
To improve test coverage, the same two unbinned test cases used for the ASAD method have been added to the Stokes method test suite, in addition to the existing no-background test. These tests raise numerical errors from numpy due to square roots of negative numbers, but the current belief is that these errors stem from the very coarse response binning (nside 1) in the test cases, rather than any actual error.