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.

Sign up for GitHub

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

Jump to bottom

Absorption correction #649

Merged
merged 50 commits into from
Sep 25, 2024
Merged

Absorption correction #649

merged 50 commits into from
Sep 25, 2024

Conversation

psavery
Copy link
Collaborator

@psavery psavery commented Jun 21, 2024

This adds a physics package class along with code to perform absorption corrections due to components of the physics package or components on the detectors.

@pep8speaks
Copy link

pep8speaks commented Jun 21, 2024 •
edited
Loading

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

Line 317:80: E501 line too long (84 > 79 characters)
Line 715:34: E231 missing whitespace after ','
Line 715:49: E231 missing whitespace after ','
Line 716:17: E128 continuation line under-indented for visual indent
Line 716:31: E231 missing whitespace after ','
Line 716:43: E231 missing whitespace after ','
Line 717:17: E128 continuation line under-indented for visual indent
Line 717:29: E231 missing whitespace after ','
Line 717:42: E231 missing whitespace after ','
Line 718:17: E128 continuation line under-indented for visual indent
Line 718:30: E231 missing whitespace after ','
Line 718:48: E231 missing whitespace after ','
Line 719:17: E128 continuation line under-indented for visual indent
Line 719:33: E231 missing whitespace after ','
Line 719:47: E231 missing whitespace after ','
Line 720:17: E128 continuation line under-indented for visual indent
Line 720:28: E231 missing whitespace after ','
Line 720:41: E231 missing whitespace after ','
Line 721:17: E128 continuation line under-indented for visual indent
Line 721:29: E231 missing whitespace after ','
Line 721:42: E231 missing whitespace after ','
Line 722:17: E128 continuation line under-indented for visual indent
Line 722:29: E231 missing whitespace after ','
Line 722:42: E231 missing whitespace after ','
Line 723:17: E128 continuation line under-indented for visual indent
Line 723:32: E231 missing whitespace after ','
Line 723:45: E231 missing whitespace after ','
Line 724:17: E128 continuation line under-indented for visual indent
Line 724:30: E231 missing whitespace after ','
Line 724:43: E231 missing whitespace after ','
Line 725:17: E128 continuation line under-indented for visual indent
Line 725:29: E231 missing whitespace after ','
Line 725:41: E231 missing whitespace after ','
Line 726:17: E128 continuation line under-indented for visual indent
Line 726:28: E231 missing whitespace after ','
Line 726:41: E231 missing whitespace after ','
Line 727:17: E128 continuation line under-indented for visual indent
Line 727:30: E231 missing whitespace after ','
Line 727:42: E231 missing whitespace after ','
Line 728:17: E128 continuation line under-indented for visual indent
Line 728:29: E231 missing whitespace after ','
Line 728:44: E231 missing whitespace after ','
Line 729:17: E128 continuation line under-indented for visual indent
Line 729:30: E231 missing whitespace after ','
Line 729:43: E231 missing whitespace after ','
Line 730:17: E128 continuation line under-indented for visual indent
Line 730:29: E231 missing whitespace after ','
Line 730:44: E231 missing whitespace after ','
Line 731:17: E128 continuation line under-indented for visual indent
Line 731:30: E231 missing whitespace after ','
Line 731:43: E231 missing whitespace after ','
Line 732:17: E128 continuation line under-indented for visual indent
Line 732:28: E231 missing whitespace after ','
Line 732:43: E231 missing whitespace after ','
Line 733:17: E128 continuation line under-indented for visual indent
Line 733:34: E231 missing whitespace after ','
Line 733:48: E231 missing whitespace after ','
Line 734:17: E128 continuation line under-indented for visual indent
Line 734:30: E231 missing whitespace after ','
Line 734:46: E231 missing whitespace after ','
Line 735:17: E128 continuation line under-indented for visual indent
Line 735:28: E231 missing whitespace after ','
Line 735:41: E231 missing whitespace after ','
Line 736:17: E128 continuation line under-indented for visual indent
Line 736:29: E231 missing whitespace after ','
Line 736:45: E231 missing whitespace after ','
Line 737:17: E128 continuation line under-indented for visual indent
Line 737:32: E231 missing whitespace after ','
Line 737:46: E231 missing whitespace after ','
Line 738:17: E128 continuation line under-indented for visual indent
Line 738:30: E231 missing whitespace after ','
Line 738:45: E231 missing whitespace after ','
Line 739:17: E128 continuation line under-indented for visual indent
Line 739:32: E231 missing whitespace after ','
Line 739:44: E231 missing whitespace after ','
Line 740:17: E128 continuation line under-indented for visual indent
Line 740:29: E231 missing whitespace after ','
Line 740:43: E231 missing whitespace after ','
Line 741:17: E128 continuation line under-indented for visual indent
Line 741:33: E231 missing whitespace after ','
Line 741:46: E231 missing whitespace after ','
Line 742:17: E128 continuation line under-indented for visual indent
Line 742:28: E231 missing whitespace after ','
Line 742:43: E231 missing whitespace after ','
Line 743:17: E128 continuation line under-indented for visual indent
Line 743:28: E231 missing whitespace after ','
Line 743:40: E231 missing whitespace after ','
Line 743:52: E231 missing whitespace after ','
Line 744:17: E128 continuation line under-indented for visual indent
Line 744:28: E231 missing whitespace after ','
Line 744:43: E231 missing whitespace after ','
Line 745:17: E128 continuation line under-indented for visual indent
Line 745:31: E231 missing whitespace after ','
Line 745:43: E231 missing whitespace after ','
Line 746:17: E128 continuation line under-indented for visual indent
Line 746:28: E231 missing whitespace after ','
Line 746:40: E231 missing whitespace after ','
Line 747:17: E128 continuation line under-indented for visual indent
Line 752:25: E231 missing whitespace after ','
Line 752:41: E231 missing whitespace after ','
Line 752:53: E231 missing whitespace after ','
Line 753:21: E231 missing whitespace after ','
Line 753:31: E231 missing whitespace after ','
Line 753:45: E231 missing whitespace after ','
Line 754:25: E231 missing whitespace after ','
Line 754:38: E231 missing whitespace after ','
Line 754:50: E231 missing whitespace after ','
Line 755:21: E231 missing whitespace after ','
Line 755:32: E231 missing whitespace after ','
Line 755:43: E231 missing whitespace after ','
Line 755:53: E231 missing whitespace after ','
Line 756:26: E231 missing whitespace after ','
Line 756:37: E231 missing whitespace after ','
Line 756:48: E231 missing whitespace after ','
Line 757:23: E231 missing whitespace after ','
Line 757:33: E231 missing whitespace after ','
Line 757:44: E231 missing whitespace after ','
Line 757:55: E231 missing whitespace after ','
Line 758:21: E231 missing whitespace after ','
Line 758:33: E231 missing whitespace after ','
Line 758:44: E231 missing whitespace after ','
Line 758:55: E231 missing whitespace after ','
Line 759:23: E231 missing whitespace after ','
Line 759:35: E231 missing whitespace after ','
Line 759:47: E231 missing whitespace after ','
Line 759:58: E231 missing whitespace after ','
Line 760:23: E231 missing whitespace after ','
Line 760:34: E231 missing whitespace after ','
Line 760:45: E231 missing whitespace after ','
Line 761:22: E231 missing whitespace after ','
Line 761:34: E231 missing whitespace after ','
Line 761:45: E231 missing whitespace after ','
Line 762:23: E231 missing whitespace after ','
Line 762:36: E231 missing whitespace after ','
Line 762:48: E231 missing whitespace after ','
Line 763:22: E231 missing whitespace after ','
Line 763:33: E231 missing whitespace after ','
Line 763:45: E231 missing whitespace after ','
Line 764:21: E231 missing whitespace after ','
Line 764:34: E231 missing whitespace after ','
Line 764:46: E231 missing whitespace after ','
Line 765:23: E231 missing whitespace after ','
Line 765:35: E231 missing whitespace after ','
Line 765:46: E231 missing whitespace after ','
Line 766:23: E231 missing whitespace after ','
Line 766:35: E231 missing whitespace after ','
Line 766:47: E231 missing whitespace after ','
Line 767:23: E231 missing whitespace after ','
Line 767:35: E231 missing whitespace after ','
Line 767:47: E231 missing whitespace after ','
Line 768:23: E231 missing whitespace after ','
Line 768:34: E231 missing whitespace after ','
Line 768:46: E231 missing whitespace after ','
Line 769:23: E231 missing whitespace after ','
Line 769:34: E231 missing whitespace after ','
Line 769:46: E231 missing whitespace after ','
Line 770:23: E231 missing whitespace after ','
Line 770:35: E231 missing whitespace after ','
Line 770:46: E231 missing whitespace after ','
Line 771:23: E231 missing whitespace after ','
Line 771:35: E231 missing whitespace after ','
Line 771:46: E231 missing whitespace after ','
Line 772:22: E231 missing whitespace after ','
Line 772:36: E231 missing whitespace after ','
Line 772:47: E231 missing whitespace after ','
Line 773:23: E231 missing whitespace after ','
Line 773:36: E231 missing whitespace after ','
Line 773:48: E231 missing whitespace after ','
Line 774:23: E231 missing whitespace after ','
Line 774:36: E231 missing whitespace after ','
Line 774:48: E231 missing whitespace after ','
Line 775:23: E231 missing whitespace after ','

Line 8:18: E203 whitespace before ':'
Line 9:20: E203 whitespace before ':'
Line 9:25: E261 at least two spaces before inline comment
Line 13:18: E203 whitespace before ':'
Line 14:20: E203 whitespace before ':'
Line 14:25: E261 at least two spaces before inline comment
Line 17:1: E305 expected 2 blank lines after class or function definition, found 1
Line 20:19: E261 at least two spaces before inline comment
Line 25:76: E261 at least two spaces before inline comment
Line 25:80: E501 line too long (82 > 79 characters)
Line 26:22: E261 at least two spaces before inline comment
Line 27:27: E261 at least two spaces before inline comment
Line 27:28: E262 inline comment should start with '# '
Line 37:32: E261 at least two spaces before inline comment
Line 40:33: E261 at least two spaces before inline comment
Line 54:27: E203 whitespace before ':'
Line 55:27: E203 whitespace before ':'
Line 55:34: E261 at least two spaces before inline comment
Line 56:28: E203 whitespace before ':'
Line 56:35: E261 at least two spaces before inline comment
Line 57:26: E203 whitespace before ':'
Line 57:35: E261 at least two spaces before inline comment
Line 60:27: E203 whitespace before ':'
Line 61:27: E203 whitespace before ':'
Line 61:34: E261 at least two spaces before inline comment
Line 62:28: E203 whitespace before ':'
Line 62:34: E261 at least two spaces before inline comment
Line 63:26: E203 whitespace before ':'
Line 63:35: E261 at least two spaces before inline comment

Line 124:53: W291 trailing whitespace
Line 152:58: W291 trailing whitespace
Line 157:55: W291 trailing whitespace
Line 167:10: E221 multiple spaces before operator
Line 178:28: E221 multiple spaces before operator
Line 184:28: E221 multiple spaces before operator

Line 292:80: E501 line too long (80 > 79 characters)
Line 1699:80: E501 line too long (97 > 79 characters)
Line 1704:43: E231 missing whitespace after ','
Line 1704:46: E231 missing whitespace after ','
Line 1715:80: E501 line too long (85 > 79 characters)
Line 1723:80: E501 line too long (86 > 79 characters)
Line 1724:55: E261 at least two spaces before inline comment
Line 1725:67: E261 at least two spaces before inline comment
Line 1725:80: E501 line too long (82 > 79 characters)
Line 1732:80: E501 line too long (86 > 79 characters)
Line 1733:55: E261 at least two spaces before inline comment
Line 1734:67: E261 at least two spaces before inline comment
Line 1734:80: E501 line too long (82 > 79 characters)
Line 1737:80: E501 line too long (95 > 79 characters)
Line 1746:19: E128 continuation line under-indented for visual indent
Line 1749:23: E231 missing whitespace after ','
Line 1762:34: E261 at least two spaces before inline comment

Line 3:5: E128 continuation line under-indented for visual indent
Line 116:1: E302 expected 2 blank lines, found 1

Line 2088:80: E501 line too long (84 > 79 characters)
Line 2108:80: E501 line too long (92 > 79 characters)

Line 105:80: E501 line too long (101 > 79 characters)
Line 128:10: E221 multiple spaces before operator
Line 141:28: E221 multiple spaces before operator

Line 4:1: E122 continuation line missing indentation or outdented
Line 14:1: E302 expected 2 blank lines, found 0
Line 28:1: E302 expected 2 blank lines, found 1
Line 30:49: W291 trailing whitespace
Line 35:10: E231 missing whitespace after ','
Line 40:1: E305 expected 2 blank lines after class or function definition, found 1
Line 46:1: E302 expected 2 blank lines, found 0
Line 46:38: W291 trailing whitespace
Line 52:1: E302 expected 2 blank lines, found 1
Line 52:48: W291 trailing whitespace
Line 75:54: W291 trailing whitespace
Line 87:19: E222 multiple spaces after operator
Line 98:10: E221 multiple spaces before operator
Line 98:18: E231 missing whitespace after ','
Line 99:25: E231 missing whitespace after ','
Line 110:31: E261 at least two spaces before inline comment
Line 111:19: E261 at least two spaces before inline comment
Line 116:1: E302 expected 2 blank lines, found 1
Line 116:48: W291 trailing whitespace
Line 137:54: W291 trailing whitespace
Line 149:19: E222 multiple spaces after operator
Line 160:10: E221 multiple spaces before operator
Line 160:18: E231 missing whitespace after ','
Line 161:25: E231 missing whitespace after ','
Line 172:31: E261 at least two spaces before inline comment
Line 173:19: E261 at least two spaces before inline comment

Comment last updated at 2024-09-25 19:23:38 UTC

@codecov Codecov
Copy link

codecov bot commented Jul 3, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 80.29661% with 93 lines in your changes missing coverage. Please review.

Project coverage is 37.34%. Comparing base (e8b3cb7) to head (b528b11).
Report is 51 commits behind head on master.

Files with missing lines Patch % Lines
hexrd/instrument/physics_package.py 77.01% 37 Missing ⚠️
hexrd/instrument/cylindrical_detector.py 5.88% 32 Missing ⚠️
hexrd/instrument/detector_coatings.py 81.31% 17 Missing ⚠️
hexrd/instrument/hedm_instrument.py 85.00% 3 Missing ⚠️
hexrd/material/utils.py 95.00% 3 Missing ⚠️
hexrd/instrument/detector.py 98.33% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #649      +/-   ##
==========================================
+ Coverage   36.41%   37.34%   +0.93%     
==========================================
  Files         130      134       +4     
  Lines       21275    21737     +462     
==========================================
+ Hits         7747     8118     +371     
- Misses      13528    13619      +91
Flag Coverage Δ
37.34% <80.29%> (+0.93%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

psavery
psavery commented Jul 3, 2024
View reviewed changes
setup.py
@@ -12,6 +12,7 @@

install_reqs = [
'appdirs',
'chemparse',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We will need to add chemparse as a conda dependency as well.

@bnmajor bnmajor force-pushed the absorption-correction branch 3 times, most recently from 76d8b5b to e8616b4 Compare September 20, 2024 13:36
@saransh13 @psavery
New packaging for performing absorption corrections
75cef15
@saransh13 @psavery
define some properties.
7785d0c
@saransh13 @psavery
added some description
1b02400
@saransh13 @psavery
start adding code for sample+window absorption correction
540c7fc
@saransh13 @psavery
add more code. almost done with this code now
036e0d9
@saransh13 @psavery
adding final few attributes
08454ec
@saransh13 @psavery
filterpack tested. still. need to test sample and window absorptions
5953137
@saransh13 @psavery
functions work, but does not look physical at the moment
430ebba
@saransh13 @psavery
fixed the transformation
b74b437
@saransh13 @psavery
many more helper routines to compute the absorption length based on f…
8b0b6de 
…ormula and density. Tested and fixed absorption correction for filterpack and sample+window layer.
@saransh13 @psavery
switching back to the old Anomalous.h5 file.
57b34bb
@saransh13 @psavery
filter and coating definitions are now on the detector class itself.
3a86f0b
@saransh13 @psavery
fixed the signs and moved all absorption calculations to the instrume…
00f5a99 
…nt class
@saransh13 @psavery
everything in the instrument class now. everything runs. coating+filt…
bf67fd9 
…er correction tested. physics package correction needs to be tested.
@saransh13 @psavery
calculate overall transmission
e6cb3a1
@saransh13 @psavery
add effective pinhole area correction
4188f66
@saransh13 @psavery
added phosphor absorption
c1e1642
@saransh13 @psavery
fix typo
f5e3fb4
@saransh13 @psavery
new class for sample, pinhole, filter, coatings etc.
1abbf08
@saransh13 @psavery
make everything tie back to the abstractlayer class, and add absorpti…
f62e751 
…on length
@saransh13 @psavery
tested code for physics package
6233315
@saransh13 @psavery
tested initialization of instrument with default values. Still need t…
a7af3fc 
…o test the transmission functions
@saransh13 @psavery
tested transmission functions.
0c4bf36
Brianna Major and others added 22 commits September 25, 2024 13:14
@psavery
Correct const capitalization
056d02a 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Minor code cleanup
e96980c 
Use pythonic formatting/naming, remove unused code.

Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Add chemparse to conda requirements
171d952 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Do not use global defaults
c7fc097 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Move contants to their own file
55213d7 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Combine pinhole with physics package
57454b3 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Move physics package to instrument level
caac83e 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Rename sample.py -> physics_package.py; move to instrument
d902388 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Separate physics package from detector coatings
a2da4c0 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Allow physics package to be None
907fd5a 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Remove left over debug statement
0ff675f 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Set physics package and detecor coatings from config if provided
687add5 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Add missing setters for phosphor
137da0e 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Explicitly list attributes to serialize
df313e7 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Fix default arguments for physics package classes
d1a3b30 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Fix some incorrectly resolved conflicts
7168d35 
Remove old check accidentally reintroduced in rebase. Fix naming
anglesToDVec -> angles_to_dvec.

Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Keep all resource files in HEXRD
530d497 
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
@psavery
Disable codecov GitHub annotations
cd721e0 
These are not really that helpful, and make PRs difficult to read.

If we want to see the coverage, we should just go to the codecov
website.

Signed-off-by: Patrick Avery <patrick.avery@kitware.com>
@psavery
Add some type annotations and improve docstring
ed5fd60 
Signed-off-by: Patrick Avery <patrick.avery@kitware.com>
@psavery
Use AbstractPhysicsPackage for type annotations
5c1ed3d 
Signed-off-by: Patrick Avery <patrick.avery@kitware.com>
@psavery
Add a test for absorption correction
126001f 
This test compares the computed, normalized transmission with an
expected one.

Signed-off-by: Patrick Avery <patrick.avery@kitware.com>
@psavery
Add some annotations and alphabetize dependencies
2cabfe9 
Signed-off-by: Patrick Avery <patrick.avery@kitware.com>
@psavery
Pin h5py version to less than 3.12
b528b11 
3.12 is having an import error on Windows, so pin to less than
3.12 until that is fixed.

Signed-off-by: Patrick Avery <patrick.avery@kitware.com>
@psavery psavery marked this pull request as ready for review September 25, 2024 19:24
saransh13
saransh13 reviewed Sep 25, 2024
View reviewed changes
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.

Some minor issues with distortion which we will fix later. Otherwise LGTM

@psavery psavery merged commit 6540509 into master Sep 25, 2024
7 checks passed
@psavery psavery deleted the absorption-correction branch September 25, 2024 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saransh13 saransh13 saransh13 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@psavery @pep8speaks @saransh13