pisa
pisa copied to clipboard
`reco.resolutions` params violate PISA "rules" for params
Modifying a param should modify the events accordingly, but reco.resolutions changes the events in a way that doesn't allow this behavior. Either the resolutions changes should be instantiation args (which can have such behaviors) or the handling of the params needs to be modified to follow the PISA rules for params. (this breaks PISA in odd ways in many places)
We should also verify that everyone developing stages/services for PISA is aware of these assumptions and has made the right choice between what is a class instantiation arg vs. a param
To be specific, the issue with reco.resolutions
is that its param values are only taken into account at setup time, and any later changes would silently be ignored, is that correct?
It appears that is correct, as the computation using params is done in setup_function
which (presumably?) will only be called once. It is also the case that the logic would need to be changed if it were called every time a param changed, since the computation depends on the current difference between reco and true values.
we should probably in such a case a require such parameters to be fixed, and b) if a fixed parameter in a stage/pipeline changed re-initialize the stage/pipeline
This increases complexity, though, doesn't it, both in the code and for developers to wrap their minds around? Currently, we have:
-
init args (i.e. every argument to the class's init besides
params
). These do something exactly once upon instantiation of the service. That's an "API contract" that we've long established. This also allows for anything to be passed to a service, defaults to be set, etc. - params As far as the service is concerned, if these are changed, then the change must be reflected in how the service runs. The long-established contract with users is that a param is something that, if it is set to a valid value, the service can do what is necessary to behave accordingly (whether that's in the beginning or after running a hundred times).
Params can be specified to be "fixed" or "free" in a config file; either case doesn't change how a service behaves, it only affects how a minimizer behaves (how it handles the params). So at the very least, the meaning of "fixed" or "free" is aliased to a different concept with your proposal.
What it sounds like you're proposing is that we have each service specify which params it requires to be fixed, and I'm not sure whether you imply that the user's choice of fixed vs. free in a config file should factor in how they are handled (or if there is an error thrown if it doesn't match up? or do you just have a new mechanism that allows for the user to do anything, but it re-initializes the entire pipeline if it is changed?).
I'd ask if you can justify having two mechanisms for what seem to be doing the exact same thing (note if you remove "init args" then it breaks backwards compatibility, so there will necessarily be two mechanisms) and adding code complexity to achieve something that's possible already?
Maybe separately: Is there a reason to have the ability to "reinitialize a pipeline" by calling some method other than __init__
on the various services in a pipeline? If that's the case, is there an in-between kind of "(re-)initialization" whereby doing what __init__
does isn't necessary? Or do you mean to truly initialize each service (i.e., by calling the class i.e. invoking __init__
) all over again if a "fixed" param is changed?
Ties in with https://github.com/icecube/pisa/issues/559