opencadc / caom2tools

Common Archive Observation Model - data engineering tools
2 stars 13 forks source link

Filter overlap error with caom2=2.3.5 #73

Open dr-rodriguez opened 6 years ago

dr-rodriguez commented 6 years ago

I've run into an error when building a CAOM observation in caom2=2.3.5. For an observation with two filters, I create a Plane.energy.bounds object as an Interval with max/min energy wavelengths and samples for each individual filter. However, because the filter coverage overlaps, I get an error:

from caom2.shape import Interval
from caom2.shape import SubInterval

all_list = []
samples = []

start, end = 5e-7, 5e-6
samples.append(SubInterval(start, end))
all_list.append(start)
all_list.append(end)
start, end = 6e-7, 2.8e-6
samples.append(SubInterval(start, end))
all_list.append(start)
all_list.append(end)
finalStart = min(all_list)
finalEnd = max(all_list)

bounds = Interval(finalStart, finalEnd, samples=samples)

Traceback (most recent call last):
  File "/Users/drodriguez/anaconda3/envs/caom2/lib/python2.7/site-packages/IPython/core/interactiveshell.py", line 2881, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-41-9262c1aac55b>", line 1, in <module>
    bounds = Interval(finalStart, finalEnd, samples=samples)
  File "/Users/drodriguez/data/CAOM/pyCAOM2/caom2/shape.py", line 255, in __init__
    self.validate()
  File "/Users/drodriguez/data/CAOM/pyCAOM2/caom2/shape.py", line 353, in validate
    'sample:\n{}\nvs\n{}'.format(sample, prev))
AssertionError: invalid interval: sample overlaps previous sample:
SubInterval.lower : 6e-07
SubInterval.upper : 2.8e-06
vs
SubInterval.lower : 5e-07
SubInterval.upper : 5e-06

In this case, one filter is entirely within another so perhaps the error is valid, but this makes it sound like any overlapping filters will give an error and this can be quite common. Or are SubInterval samples meant to be used in a different way?

pdowler commented 6 years ago

SubInterval samples are intended to be separate (have holes between them). In our other code (java) we simply merge overlapping SubIntervals while adding them to the list. As an aside, the same could be said for MultiPolygon: there is an implicit assumption that all samples that can be merged (union) into a single "loop" have been and the remaining samples are really disjoint. It probably is not checked/enforced.

Since we started out having a single standard library that computed Interval(s) and MultiPolygon(s), these validation checks grew out of what were essentially bug-detection checks in that code... some checks are more diligent than others.