pisa icon indicating copy to clipboard operation
pisa copied to clipboard

Serialized objects should store the FTYPE used to create them

Open jllanfranchi opened this issue 7 years ago • 1 comments

See issue #437 ... E.g., log binning created with FP32 and stored to disk doesn't load if running PISA in FP64 since to that precision, bin edges do not appear to be logarithmically spaced, but we'll want to be able to work with both.

There is the question of whether to convert the datatype to current PISA FTYPE upon loading an object stored using a different FTYPE. At least a warning should be provided, and the load should not fail as it does now (at least this seems reasonable to me).

While this arose with @philippeller fixing binning to actually use the chosen FTYPE, this same behavior should probably be propagated to all objects that can be serialized. (At the very least, store the FTYPE used and we can decide how to use that information later.)

The implications are pretty far-reaching, though; all objects that can be instantiated from serialized objects would need to add e.g. an optional ftype argument (that defaults to pisa.FTYPE). This doesn't seem too onerous, but it does change the interfaces for probably a bunch of objects. And then there's the issue of how to work with legacy objects that have already been serialized to disk (i.e. that don't have the ftype field in them). If simply loaded, which is done by loading the JSON and then instantiating the object essentially via **kwargs, then the ftype arg will come from the default, and could contradict the data type actually used to create the object. Any further assumptions that depend on ftype might then fail, unless we do some hacks. (E.g., the workaround for binning could be to check for log spacing in FP32 precision even if ftype is FP64.)

jllanfranchi avatar Jan 13 '18 16:01 jllanfranchi

Sure, that should probably be the case, but it didn't cause any harm, so getting rid of it for now..

LeanderFischer avatar Jun 03 '24 14:06 LeanderFischer