Create better resampling #13
Conversation
Co-authored-by: Olivia R. Lynn <[email protected]>
| All the keyword arguments, including the values needed to sample parameters. | ||
| """ | ||
| if self.host is not None: | ||
| self.host.sample_parameters(**kwargs) |
There was a problem hiding this comment.
I'm a little confused why we need a separate recipe for host and can't just do
host = PhysicalModel()
host_parameters = host.sample_parameters()
There was a problem hiding this comment.
My thinking is that when we resampled the PhysicalModel's parameters, we would always want to make sure to resampled the host's parameters as well. However if we'd prefer to separate the two, then we could use the approach you suggest. What would be the most useful from the user perspective?
There was a problem hiding this comment.
I'm still confused about why host is being referred to here as a special named part of the PhysicalModel. To me it's just another node in the DAG even if the physical meaning is special for extragalactic sources, but if it also represents the static background, then that would be a more meaningful distinction, no?
There was a problem hiding this comment.
We can definitely change the name. The basic idea behind special casing host is to allow: 1) the source to read information that isn't provided in the source (RA, dec, etc.) and 2) provide a name for sources to refer back to their host. We could rename it to something like "background" or get rid of the special casing altogether and just have it be an attribute.
Currently TDAstro requires the creation of a new
PhysicalModels anytime we resample the parameters. This might be expensive. This PR adds a slightly more flexible model where thePhysicalModelcan pull attributes samples from functions. It is still a bit awkward because the function parameters need different names.A next step will be to extend this to take in a DAG of sampling effects and do the sampling based on that DAG.