-
Notifications
You must be signed in to change notification settings - Fork 12
Generalized, polarization aware anisotropic ratio #147
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
base: main
Are you sure you want to change the base?
Generalized, polarization aware anisotropic ratio #147
Conversation
…tions-reflections' of https://github.com/usnistgov/PyHyperScattering into 133-feat-generalize-ar-calculations-chi-center-polarizations-reflections
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks solid; on to testing
img (xarray): xarray to work on | ||
chi (numeric): q about which slice should be centered, in deg | ||
chi_width (numeric): width of slice in each direction, in deg | ||
def slice_chi(self, chi, chi_width=5, do_avg: bool = True): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't the handling on this be greatly simplified by making use of the _reRange_chi function later? Or is there a reason we don't want to do that?
@@ -161,6 +484,434 @@ def collate_AR_stack(sample,energy): | |||
print(f' Pol 0: {pol0}') | |||
print(f' Pol 90: {pol90}')''' | |||
|
|||
def _reRange_chi(self, chi): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As somewhat noted above, I think this could maybe be applied more to other functions to greatly simplify them. @pbeaucage , are you married to the "nshift" handling within the slice_chi function? It might be easier to just take everything and anytime you're trying to index chi, you just change those things you're trying to index with to valid values
if (AR_para < AR_perp).all() or (AR_perp < AR_para).all(): | ||
warnings.warn('One polarization has a systematically higher/lower AR than the other. Typically this indicates bad intensity values.',stacklevel=2) | ||
# User wants to infer chi centers from beam polarization metadata | ||
if infer_chi_from_pol == True: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Elegant, I like it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good with tests
Had a conversation with @pbeaucage about the failing test_AR_unity - looking to refactor that now. |
…er-polarizations-reflections
…er-polarizations-reflections
@delongchamp I'm going thru stale PRs and I think (think) the only thing this is hung on is a validation that we expect A of a perfect sine wave to be 0.31478. I feel like you came to a conclusion on an analytical solution to this during the code camp; any chance this survived? Or if enough of us bless that as a heuristic solution, then we can just do that... I'm not picky. |
@pbeaucage I'm sorry to be the holdup here but I really want to give this one a more thorough review. IT's difficult to strike a balance between flexibility - calculating A with different strategies such as single, multiple polarization and potentially accomodating biaxiality, and transparency, letting the user know how A is/was calculated. |
Hackathon final push for Bijal's AR work.