support for new object type skysim5000 and "rough flux"#158
Conversation
JoanneBogart
force-pushed
the
u/jrbogart/skysim_support
branch
from
8012b9a to
dff60d4
Compare
jchiang87
left a comment
jchiang87
left a comment
There was a problem hiding this comment.
Looks mostly ok, but it does seem like you need to provide separate SED factories if the cosmologies for galaxy and skysim5000 can be different.
| ''' | ||
| return _load_lsst_bandpasses()[0] | ||
|
|
||
| def load_lsst_bandpasses_version(): |
There was a problem hiding this comment.
lsst_bandpasses_version seems like a better name.
| if att in self.native_columns: | ||
| return self.get_native_attribute(att) | ||
|
|
||
| if rough: # rough flux is acceptable |
There was a problem hiding this comment.
Are these "rough" fluxes always available? Or are they only available in the precomputed flux files for special cases? Some comments here (or in the docstring) explaining when they are available would be helpful.
There was a problem hiding this comment.
They are only available if precomputed during catalog creation, in which case they're in the main file. (The catalog creation script for main catalogs in the development branch of skyCatalogs_creator has a new --rough-flux option to request them.) They can't be computed by the API on the fly because they depend on access to the magnitudes from the truth catalog. Currently those magnitudes are not saved in skyCatalogs.
--rough-flux is described in the help for the script create_main.py in repo skyCatalogs_creator. When/if there is a catalog including the rough flux columns, I'll make a new doc page similar to this one which will describe the column.
There was a problem hiding this comment.
I'll put something in the docstring
There was a problem hiding this comment.
It might be useful to explain when they are available in the docstring.
| th_parameters = self._config.get_tophat_parameters(object_type=g) | ||
| cosmology[g] = self._config.get_cosmology(g) | ||
| if th_parameters: | ||
| # In the unlikely case that object types galaxy and |
There was a problem hiding this comment.
Can one set different cosmologies in the yaml config for different galaxy types? If so, then I think you need to have separate sed factories.
There was a problem hiding this comment.
The cosmologies in the configs actually come from the underlying truth catalog. They could conceivably be different, though to date I don't believe they every have been. The new code will keep them separate, just in case.
| ''' | ||
| return _load_lsst_bandpasses()[0] | ||
|
|
||
| def load_lsst_bandpasses_version(): |
There was a problem hiding this comment.
I think it would be cleaner to define a BandPasses class like this:
class BandPasses(dict):
def __init__(self, **kwargs):
super().__init__(**kwargs)
self._version = None
@property
def version(self):
return self._version
and use that in load_lsst_bandpasses instead of a plain dict, setting the version in that function and so have it carried around with the band passes dict and not have to do the _load_lsst_bandpasses / load_lsst_bandpasses workaround.
If you don't want to something like this in this PR, that's fine, but this is a much better way of handling the versioning info, IMO.
There was a problem hiding this comment.
I agree that using a class to keep the two pieces of information together is neater than the current arrangement, but I'm going to leave it as is for now.
Revamp to keep all factories in a dict, keyed by object type
| if att in self.native_columns: | ||
| return self.get_native_attribute(att) | ||
|
|
||
| if rough: # rough flux is acceptable |
There was a problem hiding this comment.
It might be useful to explain when they are available in the docstring.
2 participants
New object type
skysim5000is structurally nearly identical togalaxyexcept there are no knots. It is generated from a skysim5000 truth catalog rather than cosmodc2 galaxiesAlso allow for "rough" fluxes. These may be included in the main sky catalog for any object type where the input truth includes magnitudes, such as skysim5000. The call interface to
get_LSST_fluxhas a new keyword argumentrough. If True the routine will