DM-48591: Add multiprofit columns to the object table #1034
Conversation
| for dataset, connection in ( | ||
| ("meas", "inputCatalogMeas"), | ||
| ("forced_src", "inputCatalogForcedSrc"), | ||
| ("psfs_multiprofit", "inputCatalogPsfsMultiprofit"), |
There was a problem hiding this comment.
Does this mean the plan is to put the multiprofit PSF fits into deepCoadd_obj (since they're per-band in the same way as other existing things), but the galaxy fits enter at the transform stage?
(That'd be fine, but it might merit some commenting, and I was curious whether it was an oversight.)
There was a problem hiding this comment.
Yes. I tried to merge the Sersic fit table here as well - the same way as for ref - but the existing solution of adding multiband tables to each per-band level in the multi-index is both confusing and inefficient (I'm pretty sure it makes a separate copy per band - at least, the columns can be assigned to independently), so I'd rather just move away from it now.
There was a problem hiding this comment.
Also afaict nothing relies on deepCoadd_obj existing, and I was under the impression that we shouldn't depend on intermediate datasets unless it's necessary, which it really isn't here.
| "for each detection in deepCoadd_mergeDet.", | ||
| dimensions=("tract", "patch", "skymap"), | ||
| storageClass="SourceCatalog", | ||
| name="{coaddName}Coadd_ref" |
There was a problem hiding this comment.
Not including deepCoadd_ref information in deepCoadd_obj was not something I expected, but it makes sense when you look at the implementations of the two tasks. And being able to make changes like that without affecting the pipeline flow is a pretty compelling reason to merge WriteObjectTableTask and TransformObjectTableTask; before seeing this I wasn't sure why you were so focused on it it (my attitude before was that it wasn't obviously better either way).
We do lose the explanability of roles we had before, i.e. "this is the task that aggregates converts to Pandas, this is the task that transforms to standardized form", but that's going away anyway if the transform task is getting more connections, and "this is the task that aggregates all of the object tables into a single standardized one" is an easy to explain role as well.
There was a problem hiding this comment.
Right. I filed DM-48895 to follow up on this. Once v29 hits I think it would make sense to merge the tasks (not sure if it's best to keep the Write or Transform name - either sounds fine to me). The logical separation was fine originally, but I think now it's just adding extra pointless I/O. Also @yalsayyad mentioned some of this dates back to gen2.
I suppose it would be best to do the same thing for the Source table tasks, but I haven't looked at those as closely.
|
|
||
| # TODO: Remove in DM-??? |
There was a problem hiding this comment.
👍 to removing this! I don't think it's actually usable anymore, but would need to deprecate some configs, at least.
There was a problem hiding this comment.
48895 (linked above) is the ticket number.
taranu
force-pushed
the
tickets/DM-48591
branch
2 times, most recently
from
ccf5754 to
b64b65f
Compare
taranu
force-pushed
the
tickets/DM-48591
branch
4 times, most recently
from
9df792a to
58fce2d
Compare
taranu
force-pushed
the
tickets/DM-48591
branch
from
58fce2d to
1e0198b
Compare
No description provided.