Spectrum refactor #2689
Spectrum refactor #2689
Conversation
|
Current issue: need to move and regenerate reference/regression data. |
|
Test failures are expected until tardis-sn/tardis-refdata#85 and tardis-sn/tardis-regression-data#8 are merged. |
tardis/spectrum/base.py
Outdated
|
|
||
| hdf_name = "spectrum" | ||
|
|
||
| def __init__(self, transport_state, spectrum_frequency): |
There was a problem hiding this comment.
Consider renaming spectrum_frequency
There was a problem hiding this comment.
perhaps frequency_grid? I like names that imply a type if we're not going to annotate
|
|
||
| class SpectrumSolver(HDFWriterMixin): | ||
| hdf_properties = [ | ||
| "montecarlo_virtual_luminosity", |
There was a problem hiding this comment.
Should we be outputting this?
There was a problem hiding this comment.
I don't see why not, might at least be useful for debugging
|
*beep* *bop* Hi, human. The Click here to see your results. |
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Currently the opacity state fixture needs to be moved
Need refdata paths and generation changed.
Tests on this branch will now fail until the regression/refdata are merged.
Also renames spectrum_virtual to spectrum_virtual_packets
andrewfullard
force-pushed
the
spectrum-refactor
branch
from
5454a2b to
e8aebcc
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2689 +/- ##
==========================================
+ Coverage 36.47% 36.61% +0.14%
==========================================
Files 183 186 +3
Lines 14664 14837 +173
==========================================
+ Hits 5348 5433 +85
- Misses 9316 9404 +88 ☔ View full report in Codecov by Sentry. |
|
|
||
| class SpectrumSolver(HDFWriterMixin): | ||
| hdf_properties = [ | ||
| "montecarlo_virtual_luminosity", |
There was a problem hiding this comment.
I don't see why not, might at least be useful for debugging
tardis/spectrum/base.py
Outdated
|
|
||
| hdf_name = "spectrum" | ||
|
|
||
| def __init__(self, transport_state, spectrum_frequency): |
There was a problem hiding this comment.
perhaps frequency_grid? I like names that imply a type if we're not going to annotate
📝 Description
Type: ☣️
breaking changeThis PR moves as much of the spectrum out of the transport state as possible without also factoring out virtual packets.
🚦 Testing
How did you test these changes?
☑️ Checklist
build_docslabel