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

Add pinhole distortion options to powder overlay editor #1275

Merged
merged 21 commits into from
Sep 13, 2022

Conversation

psavery
Copy link
Collaborator

@psavery psavery commented Aug 30, 2022

This adds pinhole distortion options to the powder overlay editor to be used in calibration. These pinhole settings are being added in hexrd in HEXRD/hexrd#446.

pinhole_correction_options.mp4

Depends on: HEXRD/hexrd#446
Depends on: #1271
Fixes: #1273
Fixes: #1279

@psavery psavery force-pushed the pinhole-distortion branch 2 times, most recently from 3552ba2 to 83f5709 Compare August 31, 2022 18:32
@psavery psavery marked this pull request as ready for review August 31, 2022 19:23
@psavery psavery force-pushed the pinhole-distortion branch from 83f5709 to 6e241da Compare August 31, 2022 21:58
@pep8speaks
Copy link

pep8speaks commented Aug 31, 2022

Hello @psavery! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 287:40: E261 at least two spaces before inline comment

Line 381:21: E117 over-indented

Comment last updated at 2022-09-12 20:04:46 UTC

@joelvbernier
Copy link
Member

joelvbernier commented Sep 2, 2022

Let's try to get #1279 to this one, since it is an important one. However, I defer to you @psavery if you want to merge this up as-is and kick that to another PR.

These are not currently connected to anything, though.

Signed-off-by: Patrick Avery <[email protected]>
This adds the infrastructure to the overlays, as well as passing it
as an argument to the PowderCalibrator class.

Signed-off-by: Patrick Avery <[email protected]>
This seems to be working so far.

Signed-off-by: Patrick Avery <[email protected]>
The hexrd classes expect millimeters, but we are displaying
micrometers. Make sure we do the conversion.

Signed-off-by: Patrick Avery <[email protected]>
Signed-off-by: Patrick Avery <[email protected]>
This fixes an issue when loading a state file where overlay materials
would not have the correct energy. They would have to be toggled on
and off to get their energy fixed.

Signed-off-by: Patrick Avery <[email protected]>
It was previously `None`, which meant that it was automatically guessed.

But this adds the ability to specify it in the GUI.

Signed-off-by: Patrick Avery <[email protected]>
@psavery psavery force-pushed the pinhole-distortion branch 4 times, most recently from 8c44a59 to f74bb3f Compare September 10, 2022 22:57
If there are powder overlays present with tth distortion active, and
we are in the polar view, then some options will be present for applying
tth distortion to the polar view.

There is a checkbox where the user can select whether to apply tth
distortion to the polar view. If checked, the user must then select an
overlay (that has tth distortion) to use.

The tth distortion of the selected overlay will be applied to the polar
view, and any other overlays with tth distortion will be offset
accordingly.

Signed-off-by: Patrick Avery <[email protected]>
@psavery
Copy link
Collaborator Author

psavery commented Sep 10, 2022

Here is a video that shows the polar image tth distortion in action:

polar_tth_distortion.mp4

I think I have just a tad bit more cleaning up to do and then this PR will be ready.

Two things left to do:

  • More testing to ensure the combo box is always updated when it is supposed to be
  • Try to avoid updating the combo box for every overlay property modification (for better performance, especially for interactive widgets)

This is necessary so it will match its key in the
`HEDMInstrument.detectors` dict, which is expected in some parts of the
code.

Signed-off-by: Patrick Avery <[email protected]>
This will improve the image mode widget so that it does not have to
update its list of possible distortion overlays (in the polar view)
for every single modification of the overlays.

Now, it only updates its list when it actually needs to.

Signed-off-by: Patrick Avery <[email protected]>
If one option becomes unavailable and we switch automatically to another
option, ensure that gets saved to the config as well, so that the canvas
will reflect the combo box option that is currently displayed.

Signed-off-by: Patrick Avery <[email protected]>
A deep render is not needed. Perform a shallow render instead.

Also, only render if a change actually occurred.

Signed-off-by: Patrick Avery <[email protected]>
The id() of the data was not always consistently working. Since overlay
names are now guaranteed to be unique, just use them instead.

Signed-off-by: Patrick Avery <[email protected]>
When the polar tth distortion is modified, don't update the overlays
preemptively. Instead, let them be updated in the polar render.

This allows them to be updated at the same time as polar render
modifications.

Signed-off-by: Patrick Avery <[email protected]>
This is for convenience, so that the default name will stay up to date
with the material name.

Signed-off-by: Patrick Avery <[email protected]>
@psavery
Copy link
Collaborator Author

psavery commented Sep 12, 2022

@joelvbernier I think this is now ready for review! Everything I have tested seems to be working.

If a distortion is applied but the source distance is infinite, the
distortion will have no effect. Pop up a warning if this is the case
indicating to the user thus.

Signed-off-by: Patrick Avery <[email protected]>
Only render on zoom width change if the mouse data is valid. This
avoids an exception from occurring.

Signed-off-by: Patrick Avery <[email protected]>
For a better user experience...

Signed-off-by: Patrick Avery <[email protected]>
@psavery psavery mentioned this pull request Sep 12, 2022
This displays on the x-axis whether the x-axis is being corrected
or not, and if it is corrected, what the standoff is.

Signed-off-by: Patrick Avery <[email protected]>
@joelvbernier joelvbernier merged commit 566ed97 into HEXRD:master Sep 13, 2022
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.

Need a passthrough for FWHM geuss in automatic peak fitting dialog Add pinhole distortion to powder overlays
3 participants