Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialized objects should store the FTYPE used to create them #438

Open
jllanfranchi opened this issue Jan 13, 2018 · 1 comment
Open

Serialized objects should store the FTYPE used to create them #438

jllanfranchi opened this issue Jan 13, 2018 · 1 comment

Comments

@jllanfranchi
Copy link
Contributor

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.)

@LeanderFischer
Copy link
Collaborator

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

@LeanderFischer LeanderFischer closed this as not planned Won't fix, can't repro, duplicate, stale Jun 3, 2024
@JanWeldert JanWeldert reopened this Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants