Provenances are made wrong in detection, wcs, zp

[rknop]

Since the change to add wcs and zp into the extraction provenance, provenances are now made in make_provenance_tree in top_level.py, and equivalently in test fixtures.

However, if you call something like Detector.run(), it has this line:

prov = ds.get_provenance('detection', self.pars.get_critical_pars(), session=session)

If you haven't already ahead of time done something to make sure that the datastore you're running on has properly created provenances, this is going to create a provenance for 'detection' that just has the detection parameters, not the full extraction parameters with dictionaries for sources, wcs, and zp. (In DataStore.get_provenance, pars_dict is passed on directly as the parameters of the provenance, whereas top_level.Pipeline.make_provenance_tree uses get_critical_pars to make sure siblings are properly defined.

Probably the solution is to do something in DataStore.get_provenance similar to what's done in top_level.Pipeline.make_provenance, but I'm not immediately conversant enough with provenances to be sure I will do it right.

[rknop]

...OK, while the problem still exists, I think I have gotten the reason why it's not broken from top_level.py wrong. top_level.py has the following:

        # make sure when calling get_critical_pars() these objects will produce the full, nested dictionary             
        siblings = {
            'sources': self.extractor.pars,
            'bg': self.backgrounder.pars,
            'wcs': self.astrometor.pars,
            'zp': self.photometor.pars,
        }
        self.extractor.pars.add_siblings(siblings)
        self.backgrounder.pars.add_siblings(siblings)
        self.astrometor.pars.add_siblings(siblings)
        self.photometor.pars.add_siblings(siblings)

I believe this is what makes things come out right. Somehow.

The whole thing is a bit tangled right now. Ideally, DataStore.get_provenance() should do the right thing even if everything's not carefully primed.

Or, perhaps another way to think about it, Detector, Backgrounder, AstroCalibrator, and PhotCalibrator all use the Parameters infrastructure, but none of them can run properly without an object of each of the other three classes existing, and add_siblings having been called on it. However, they will run without errors, they just will do wrong things. This suggests that they should either all be one object, or that there should be some linkage object that they all should use to do the siblings thing, throwing an error if all the linkages aren't present.

[guynir42]

Yes, that's a good point. I think there should be an informative error thrown by the Parameters object on any of these things, where it will not allow you to access get_critical_parameters() unless the siblings are defined (the siblings inside the Parameters object is the link you are looking for).

perhaps there should be a function on Parameters that's called must_have_siblings and on some subclasses that returns True, and if it is true will throw an exception when you call get_critical_parameters().