Flexible packet initial radius #1592
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1592 +/- ##
==========================================
- Coverage 67.20% 67.20% -0.01%
==========================================
Files 73 73
Lines 6147 6150 +3
==========================================
+ Hits 4131 4133 +2
- Misses 2016 2017 +1
Continue to review full report at Codecov.
|
| @@ -166,7 +166,7 @@ def _initialize_geometry_arrays(self, model): | |||
| self.r_outer_cgs = model.r_outer.to("cm").value | |||
| self.v_inner_cgs = model.v_inner.to("cm/s").value | |||
|
|
|||
| def _initialize_packets(self, T, no_of_packets, iteration): | |||
| def _initialize_packets(self, T, no_of_packets, iteration, radius): | |||
There was a problem hiding this comment.
Not really related to this PR but maybe we should rename T (e.g. temperature).
There was a problem hiding this comment.
Agreed. For the restructuring to come!
| @@ -175,10 +175,11 @@ def _initialize_packets(self, T, no_of_packets, iteration): | |||
| seed = self.seed + iteration | |||
| rng = np.random.default_rng(seed=seed) | |||
| seeds = rng.choice(MAX_SEED_VAL, no_of_packets, replace=True) | |||
| nus, mus, energies = self.packet_source.create_packets( | |||
| T, no_of_packets, rng | |||
| radii, nus, mus, energies = self.packet_source.create_packets( | |||
There was a problem hiding this comment.
I am wondering if the radius/radii should be an attribute of the packet_source. In the case of the BlackBodySimpleSource, it would be set to r_inner by default. This way we might avoid passing this information through multiple layers.
There was a problem hiding this comment.
Possibly, but we also expect the radius to be different for different packets in the nebular phase. So it needs to be an array of radii corresponding to the energy deposition radius.
There was a problem hiding this comment.
I think there are all sorts of potential ways to configure this - but for now I think it's fine how it is.
| @@ -166,7 +166,7 @@ def _initialize_geometry_arrays(self, model): | |||
| self.r_outer_cgs = model.r_outer.to("cm").value | |||
| self.v_inner_cgs = model.v_inner.to("cm/s").value | |||
|
|
|||
| def _initialize_packets(self, T, no_of_packets, iteration): | |||
| def _initialize_packets(self, T, no_of_packets, iteration, radius): | |||
| @@ -175,10 +175,11 @@ def _initialize_packets(self, T, no_of_packets, iteration): | |||
| seed = self.seed + iteration | |||
| rng = np.random.default_rng(seed=seed) | |||
| seeds = rng.choice(MAX_SEED_VAL, no_of_packets, replace=True) | |||
| nus, mus, energies = self.packet_source.create_packets( | |||
| T, no_of_packets, rng | |||
| radii, nus, mus, energies = self.packet_source.create_packets( | |||
There was a problem hiding this comment.
I think there are all sorts of potential ways to configure this - but for now I think it's fine how it is.
* Packet collections are initialized with a radius * Docstring for create_packets
Description
The PacketCollection now includes the initial radius of packets. For the BlackBodySource this is just the inner radius of the ejecta.
Motivation and context
Preparation for creating packets without the inner radius set explicitly to the model inner radius e.g. for nebular phase
How has this been tested?
Examples
Type of change
Checklist