search

LSSTDESC / descwl-coadd-task

DM Task to run coaddition in cells
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

Exclude partially overlapping calexps #2

Closed arunkannawadi closed 2 years ago

esheldon commented 3 years ago

Arun, this is marked as a draft. Is there more work to be done on this?

esheldon commented 3 years ago

I think that a bbox can check contains on another bbox

bbox = geom.Box2I( geom.Point2I(10, 10), geom.Point2I(50, 50))
bbox2 = geom.Box2I( geom.Point2I(20, 20), geom.Point2I(30, 30))
bbox.contains(bbox2)
True

can we use this to simplify the code?

esheldon commented 3 years ago

I don't know if sky bboxes can do that check

esheldon commented 3 years ago

I'm getting this error:

  File "/home/esheldon/git/descwl-coadd-task/python/lsstdesc/pipe/task/coadd_in_cells.py", line 79, in run
    hash_key = hash_function(self.config.seed, skyInfo.tract, skyInfo.patch, calExpList[0].dataId["band"])
AttributeError: 'Struct' object has no attribute 'tract'
beckermr commented 3 years ago

We really need CI tests on these tools. Not sure how DM does that.

arunkannawadi commented 3 years ago

Arun, this is marked as a draft. Is there more work to be done on this?

I wanted to write a unittest before marking it as ready to review. That's why it's a draft currently.

I don't know if sky bboxes can do that check

Yeah, in sky coordinates, bboxs have curved edges unless we linearize the WCS. I figured it's easier to deal with corners, and computationally would also be more effective in checking if it is inside as opposed to transforming a whole bbox.

We really need CI tests on these tools. Not sure how DM does that.

Of course, we will need them soon. DM has a small dataset of HSC and DC2 images, which is used to CI (ci_hsc and ci_imsim). It is run on Jenkins, but can also be copied locally and run anywhere. Let's discuss on how to setup a CI workflow that with these datasets before we merge it to the DM codebase.

arunkannawadi commented 3 years ago

Agree. I figured computing the bounding box was not computationally expensive and could be used. Is it preferable to pass this via a config because it will ensure it stays the same for all the cells? And if this parameter is configured, I don't think it makes sense for the make_inputs function to return it.

arunkannawadi commented 2 years ago

The code has drastically differed (and become shorter) than what this PR started with. Closing this and opening another one. All the comments here have been considered and included in the new branch.