Add versioned TensorMap save and load - #512
Conversation
|
The Codex implementation appears to follow the spirit of what @lkdvos outlined here (#212 (comment)). |
Codecov Report❌ Patch coverage is
... and 6 files with indirect coverage changes 🚀 New features to boost your workflow:
|
lkdvos
left a comment
There was a problem hiding this comment.
Great to see this get started. I think I have two overall comments here:
The first one considers whether we could just have the JLD2 dependency as a package extension. I definitely think it is good to provide native support for JLD2, but I think this can live in an extension to avoid pulling in that dependency whenever it isn't needed. I would have to investigate a bit more in detail how to make this reachable in that case, but one sidenote here is that exporting save and load is probably not something we can do, as these are very generic names to put in the namespace and we probably will introduce nameclashes here.
The second one is a larger one that pops up in a variety of ways throughout this PR, which mostly considers how far we want to go with the longevity of the stored data. In general, the rule is to avoid storing custom datatypes altogether, and stay with simple Int, Char, String, Float.. etc representations. The main point here is that any saved struct is a possible point for breaking, either through renaming, reparametrizations or anything else.
The Dict conversion was designed specifically to handle this, which looks like what Codex based its storage schemes on, but it got rid of the string representation of the sectors, therefore explicitly baking in the sectortypes into the storage schemes and coupling them again. I do feel like if we are going to write custom storage schemes, it is better to immediately get a native-types-only representation going, since otherwise there is not that much to gain over simply storing the data using the default JLD2 machinery, which would already handle this.
In a somewhat similar line of reasoning, it does look like this storage scheme is still coupled to our specific data representation in a somewhat implicit way - the coupled sectors and matrix blocks lay out the data in an order that is just a convention that we chose, and can silently change between versions (as happened before in v0.13 IIRC). In other words, for a truly self-descriptive data scheme, it would be better to store fusiontree-subblock pairs explicitly, as this now no longer depends on this internal choice we made.
The PR is authored by Codex, and I haven't carefully review it. So it will remain draft for a while.
This PR adds standardized, versioned JLD2 IO for TensorKit tensor maps through exported
save(path, tensor)andload(path)functions. Closes #211.TensorMap,Tensor,DiagonalTensorMap, andBraidingTensor.AdjointTensorMapobjects to be materialized before saving.The focused IO tests pass 59/59, and the new dense format produces smaller files than saving the legacy Dict representation with JLD2.