DM-29279: Add healsparse input map generation to AssembleCoaddTask #494
Conversation
| @@ -99,6 +100,12 @@ class AssembleCoaddConnections(pipeBase.PipelineTaskConnections, | |||
| storageClass="ImageU", | |||
| dimensions=("tract", "patch", "skymap", "band"), | |||
| ) | |||
| inputMap = pipeBase.connectionTypes.Output( | |||
There was a problem hiding this comment.
Oh man, I'm going to have to get used to inputMap being an Output
There was a problem hiding this comment.
If you think this should have another name, now is the time to yelp!
| dtype=int, | ||
| default=32768, | ||
| ) | ||
| nside_coverage = pexConfig.Field( |
There was a problem hiding this comment.
Oh man, I'm not ready for a mix of snake and camel case configs.
There was a problem hiding this comment.
Re: snake case for configs, may I quote @TallJimbo in response to the question "Are config variable names similarly supposed to be snake in new tasks going forward?": "Config names aren't covered explicitly in the guidelines, but I'd say yes."
| class HealSparseInputMapConfig(pexConfig.Config): | ||
| """Configuration parameters for HealSparseInputMapTask""" | ||
| nside = pexConfig.Field( | ||
| doc="Mapping healpix nside. Must be power of 2.", |
There was a problem hiding this comment.
There are ways to require that its power of 2. Either here with check kwarg or in a validate method.
There was a problem hiding this comment.
And now I found a new trick of checking that an integer is a power of two. Today is a good day! https://stackoverflow.com/questions/57025836/how-to-check-if-a-given-number-is-a-power-of-two
| default=256, | ||
| ) | ||
| bad_mask_min_coverage = pexConfig.Field( | ||
| doc=("Minimum area fraction of a map healpixel pixel that must be " |
There was a problem hiding this comment.
But they do make the auto-indentation work correctly so I use them...
| self._bits_per_visit_ccd = {} | ||
| self._bits_per_visit = defaultdict(list) | ||
| for bit, ccd in enumerate(ccds): | ||
| metadata[f'B{bit:04d}CCD'] = ccd['ccd'] |
There was a problem hiding this comment.
double check that we're guaranteed to have a column called 'ccd'
There was a problem hiding this comment.
This is defined here:
and I know that I've mentioned this on slack. Right now it's hard-coded to be called"ccd" and if it isn't a whole bunch of code is going to break (including this code!). I don't know of any plans to update this to "detectorId" (it can't be "detector" because that's reserved for the ExposureCatalog).
tests/test_assembleCoadd.py
Outdated
| @@ -199,6 +198,32 @@ def __init__(self, *args, **kwargs): | |||
| DcrAssembleCoaddTask.__init__(self, *args, **kwargs) | |||
|
|
|||
|
|
|||
| # class MockInputMapAssembleCoaddConfig(MockSafeClipAssembleCoaddConfig): | |||
There was a problem hiding this comment.
Left these in by accident
tests/test_assembleCoadd.py
Outdated
| # class MockInputMapAssembleCoaddTask(MockSafeClipAssembleCoaddTask): | ||
| # class MockInputMapAssembleCoaddTask(MockAssembleCoaddTask, SafeClipAssembleCoaddTask): | ||
| class MockInputMapAssembleCoaddTask(MockCompareWarpAssembleCoaddTask): | ||
| """Lightly modified version of `SafeClipAssembleCoaddTask` |
There was a problem hiding this comment.
It is now after I fixed it! 😄
tests/test_assembleCoadd.py
Outdated
| _DefaultName = "inputMapAssembleCoadd" | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| # SafeClipAssembleCoaddTask.__init__(self, *args, **kwargs) |
| validPix, raPix, decPix = inputMap.valid_pixels_pos(return_pixels=True) | ||
|
|
||
| # Confirm that all the map pixels are in the bounding box | ||
| xPix, yPix = exposures[100].getWcs().skyToPixelArray(raPix, decPix, degrees=True) |
There was a problem hiding this comment.
anything special about expId=100?
There was a problem hiding this comment.
It's the first one, and they all have the same WCS in the tests. I've added a comment to that effect.
tests/test_assembleCoadd.py
Outdated
| @@ -262,6 +287,56 @@ def testDcrGen2Gen3Compatibility(self): | |||
| assembleTask = MockDcrAssembleCoaddTask(config=config) | |||
| self.checkGen2Gen3Compatibility(assembleTask) | |||
|
|
|||
| def testInputMapGen3(self): | |||
| import numpy as np | |||
There was a problem hiding this comment.
You mean to move this up top?
erykoff
force-pushed
the
tickets/DM-29279
branch
from
e997074
to
bc59641
Compare
erykoff
force-pushed
the
tickets/DM-29279
branch
from
bc59641
to
4542975
Compare
No description provided.