updated npol_to_mzp transformation by inverting Eq 44 #158
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #158 +/- ##
==========================================
- Coverage 92.18% 91.48% -0.70%
==========================================
Files 9 9
Lines 550 552 +2
==========================================
- Hits 507 505 -2
- Misses 43 47 +4 ☔ View full report in Codecov by Sentry. |
| if "Singular matrix" in str(err): | ||
| raise ValueError("Conversion matrix is degenerate") | ||
| else: | ||
| raise |
There was a problem hiding this comment.
What are you trying to raise here?
There was a problem hiding this comment.
If the conversion matrix turns out to be singular and inversion could not be done
There was a problem hiding this comment.
Yes. But you just have else: raise without anything actually being raised.
| conv_matrix_inv = np.linalg.inv(conv_matrix) | ||
| except np.linalg.LinAlgError as err: | ||
| if "Singular matrix" in str(err): | ||
| raise ValueError("Conversion matrix is degenerate") |
There was a problem hiding this comment.
Can we make this a custom error?
There was a problem hiding this comment.
This is derived directly from IMAX module from @lowderchris
| input_keys = list(input_collection.keys()) | ||
| input_dict = {} | ||
| in_list = list(input_collection) | ||
| phi = [input_collection[key].meta['POLAR'] for key in input_keys if key != 'alpha']*u.degree |
There was a problem hiding this comment.
I know phi comes from the paper, but it's a bit of an opaque name.
| ("Z", NDCube(Bmzp[0 * u.degree], wcs=input_collection[input_keys[0]].wcs, mask=mask, meta=metaZ)), | ||
| ("P", NDCube(Bmzp[60 * u.degree], wcs=input_collection[input_keys[0]].wcs, mask=mask, meta=metaP))] | ||
| for p_angle in in_list: | ||
| Bmzp_cube = [(key, NDCube(data_mzp_solar[:, :, i, 0], wcs=input_collection[input_keys[0]].wcs, |
There was a problem hiding this comment.
I don't really like calling this a cube. It's a list of cubes.
PR summary
What does this PR introduce?
Modifies npol_to_mzp in transforms.py by inverting Eq 44 in DeForest et al (2022)