Skip to content

Conversation

@psavery
Copy link
Collaborator

@psavery psavery commented Oct 15, 2024

This adds system relative constraints, where the entire detector system can be tilted/translated.

The center of rotation for the tilt is always about the mean center of the detectors.

This is an ideal setup if many subpanels are present, but their relative positions and tilts should remain fixed, while the entire system is allowed to calibrate.

The relative constraints for the whole system require the detectors to
all be translated and rotated together about the center of the system.

This includes a fairly thorough test.

It won't be that difficult to add relative constraints for groups too,
but that should be done in a separate branch.

Signed-off-by: Patrick Avery <[email protected]>
This is what the old tilt *actually* is...

Signed-off-by: Patrick Avery <[email protected]>
I'm not sure if this will get used, but let's add it for consistency.
It actually makes some changes elsewhere easier.

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

pep8speaks commented Oct 15, 2024

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

Line 200:5: E266 too many leading '#' for block comment
Line 250:5: E266 too many leading '#' for block comment
Line 307:5: E266 too many leading '#' for block comment

Comment last updated at 2024-10-15 21:55:16 UTC

@psavery psavery force-pushed the relative-constraints branch 3 times, most recently from 5d651b3 to b8bdd47 Compare October 15, 2024 21:38
The classes keep track of the current values of the relative parameters
(before they were modified by lmfit). This is necessary for modifying
all detectors by the diff of the change.

Signed-off-by: Patrick Avery <[email protected]>
@psavery psavery force-pushed the relative-constraints branch from b8bdd47 to ad7f741 Compare October 15, 2024 21:55
@codecov
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 78.80435% with 39 lines in your changes missing coverage. Please review.

Project coverage is 39.11%. Comparing base (45847c9) to head (ad7f741).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
hexrd/fitting/calibration/relative_constraints.py 73.68% 15 Missing ⚠️
hexrd/fitting/calibration/structureless.py 50.00% 10 Missing ⚠️
hexrd/instrument/hedm_instrument.py 42.85% 8 Missing ⚠️
hexrd/fitting/calibration/lmfit_param_handling.py 94.44% 4 Missing ⚠️
hexrd/fitting/calibration/instrument.py 90.47% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #723      +/-   ##
==========================================
+ Coverage   38.78%   39.11%   +0.33%     
==========================================
  Files         134      135       +1     
  Lines       21988    22168     +180     
==========================================
+ Hits         8527     8670     +143     
- Misses      13461    13498      +37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@saransh13 saransh13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@psavery psavery merged commit 8c669ab into master Oct 22, 2024
7 checks passed
@psavery psavery deleted the relative-constraints branch October 22, 2024 19:54
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.

4 participants