LSSTDESC / DESC_DC2_imSim_Workflow

BSD 3-Clause "New" or "Revised" License
3 stars 3 forks source link

add a faster function to find sensors to simulate in a visit #13

Closed jchiang87 closed 5 years ago

jchiang87 commented 6 years ago

@villarrealas This finds the sensors that have at least one corner within the nominal simulation region. See https://docs.google.com/document/d/18nNVImxGioQ3tcLFMRr67G_jpOzCIOdar9bjqChueQg/edit for the definition of the Run2.0 region.

Feel free to modify it. I haven't tested it against the current code.

jchiang87 commented 6 years ago

You'll need a local copy of the opsim_db file to use it.

villarrealas commented 6 years ago

I'm probably going to build a singularity image to play around with this a little and see how it works. My understanding is that what this code is designed to do is look at the geometry for DC2 and only append detectors to the sensor list IF they are within that geometry.

I'm guessing at this point we do not need to check that objects >= minsource to proceed with analysis if this works, though I think this does mean that we will be including quite a bit of edge sensors on the first pass that would not have sufficient objects (e.g. one or two corners in the simulation space because most of the detector is off the side). I'll do some testing though to see how much the two methods vary.

jchiang87 commented 6 years ago

We can add an exclusion region just inside the simulation region border to effectively reduce the simulation area to avoid sensors on the edge with small overlap with the actual simulation region. The range checks in the Run20Region.contains method would just need some modification.

jchiang87 commented 6 years ago

@danielsf If you have time, could you have a look at the trim_sensors.py script please? It's intended to find all sensors for a given visit that have any overlap with the DC2 WFD region. Thanks!

danielsf commented 6 years ago

If I may be paranoid: trim_sensors won't catch the case where the corner of the cosmoDC2 region is inside a sensor, but no corners of the sensor are inside the cosmoDC2 region. Obviously, if that happens, we won't lose much area by not simulating the sensor. If we actually care, we could just run the same if any([contains(*corner)... test testing if the corners of cosmoDC2 fall inside the sensor. That may be a lot slower thought, and we're probably not talking about a lot of visits.

jchiang87 commented 6 years ago

Thanks, Scott. I'm thinking we wouldn't care if we missed the occasional sensor on the corner, but a work around that might not be too slow would be to consider the box aligned in RA, DEC that contained each sensor, e.g., bounded by the min/max RA/DEC values, and we just check if the simulation region corners were within those bounds. We'd get the occasional "extra" corner sensor, but we wouldn't miss any, and it would be relatively cheap to compute.

danielsf commented 6 years ago

Sounds good to me

villarrealas commented 6 years ago

Just to add in a quick comment - it runs blistering faster on the log-in node compared to the old version. I think a bit of slowdown should be perfectly reasonable to deal with, if it catches some of these missing corners.

jchiang87 commented 6 years ago

@villarrealas How are you using or planning to use the new trim_sensors.py code? We should merge this branch or close it. If the latter, then I plan to move the trim_sensors.py module elsewhere (which we may want to do anyway) so that Tom can use it in the phosim pipeline.

villarrealas commented 5 years ago

Sorry for not getting to this sooner. We should get this merged ASAP for the workflow. I think the only thing that needs to be changed is this modification to instcat_trimmer, which is basically just deleting a large chunk of no longer relevant code. I am going to resolve that conflict and merge.