DM-35207: add task to produce updated visit summaries

[TallJimbo]

No description provided.

[laurenam]

This is the only PR I've really looked at so far, and most of my comments are just optional suggestions for clarifications. I'm submitting this review now, but am going to pause before approving because I'm worried enough about the conjecture:

ComputeExposureSummaryStatsTask (which was actually written to work with afw.table, but Astropy is similar enough that it works, too)

that I'm going to get all the branches set up and run a local ci_hsc to have a look at the outputs before moving on to the rest.

[laurenam]

Ok, yep, we have a problem. In my local ci_hsc run, almost all of the psf metrics have NaN entries in the finalVisitSummary catalog. I've tracked it down to this clause in ComputeExposureSummaryStatsTask:

        if (
            self.config.starSelection not in sources_columns
            or self.config.starShape + '_flag' not in sources_columns
        ):
            # The source catalog does not have the necessary fields (as in some
            # tests).
            return

noting that self.config.starShape = "base_SdssShape".

It seems that the base_SdssShape_ columns do not exist in the finalized_src_table, so this task silently returns here (my subsequent check on psfTraceRadiusDelta passes as these values did get updated before this return happens). I haven't figured out why they don't appear yet (maybe a slot default change?), but I'm not sure if the best way to fix this is to get those fields into the finalized_src_table or to somehow make sure we are only relying on the slot columns being present? @erykoff may also have an opinion here?

[And we'll also need to change the getX() and getY() usage in favor of explicit column names in maximum_nearest_psf_distance(). They didn't trigger an error because it never made it that far!]

[erykoff]

The slot nomenclature does not get propagated to parquet. You get either the slot name or the base name and now I don't know what comes through in the way I configured it. But all that alias stuff, as well as .getX() and .getY() won't work with the finalized src tables.

[laurenam]

It looks like all of the slot_ columns do get propagated to finalized_src_table (but there's no way to know which algorithm the slot refers to...)

[laurenam]

I was just wondering how any warps were getting made (i.e. selected) with all the psf metrics being NaN...seems it's because:

float("nan") > 0.009
False

so they pass the threshold test! I guess we should be checking against ~np.isfinite too?

[erykoff]

Right, you can copy over the slot names or the base names, but if you want both names you have to store twice the data. I guess I decided to make it use the slot names downstream.

[TallJimbo]

I think I've addressed all review comments except the "well, this is all silently broken" doozy at the end.

Thanks for the investigation on that, and I'll start my own where you left off. I am inclined to set the default PSF shape to the slot one down in ComputeSummaryStatisticsTask, and then obliterating whatever try/catch block made that table incompatibility a silent failure, but we'll see if there any knock-on effects.

[TallJimbo]

Yeah, back when I wrote this and looked at the usage your ticket hadn't merged yet. Smells like something is catching waaaay more exceptions than it should, and that's a real problem. I'll investigate.

[TallJimbo]

Ok, I think I've fixed the serious problem here as well, though my ci_hsc run to check that is ongoing. Pretty much all of those changes were over in pipe_tasks; here the only change is sources_columns=... -> sources_is_astropy=True to reflect changes there.

[laurenam]

pipe_tasks -> drp_tasks

[laurenam]

Garhhh...looks like ups/drp_tasks.table needs some dependencies added. Maybe just:

setupRequired(pipe_tasks)
setupRequired(skymap)