search

wtbarnes / mocksipipeline

Pipeline for producing synthetic images from the Multi-Order Spectral Imager (MOXSI)
MIT License
3 stars 1 forks source link

Introducing InstrumentDesign and OpticalDesign #25

Closed jacobdparker closed 7 months ago

jacobdparker commented 8 months ago

Introducing two new classes InstrumentDesign and OpticalDesign which make it easier to store configuration defaults and different MOXSI design iterations.

codecov-commenter commented 8 months ago

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (93e06a2) 60.18% compared to head (1753e61) 63.40%.

Files Patch % Lines
mocksipipeline/inversion/response_matrix.py 20.00% 4 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #25 +/- ## ========================================== + Coverage 60.18% 63.40% +3.21% ========================================== Files 8 13 +5 Lines 540 582 +42 ========================================== + Hits 325 369 +44 + Misses 215 213 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

wtbarnes commented 8 months ago

Looks like the tests are failing because the channel classes are instantiated differently now. Should just be a matter of fixing the tests to instantiate them correctly with an OpticalDesign and a filter choice.

wtbarnes commented 8 months ago

Alright I've reorganized things by renaming detector to instrument and moving the optical design stuff to optics. Anything that gets instantiated gets put in a configuration.py file.

Now that I'm seeing the length of one of these instrument configurations, I wonder if it makes sense to actually make instrument.configuration a subpackage and then put each configuration in its own file. The optics configurations could stay as they are since they're relatively short.

jacobdparker commented 7 months ago

Yeah, I can see that. It is very clunky that they are so long, but I do think having the math there is useful as a record. Otherwise they just seem like magic numbers.