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.
|
So you mean that processing datas with |
|
Storing the numerical data in the subblocks associated with individual fusion trees should definitely be a stable way to store the data. Of course, the difficulty is then in how to store the keys of each subblock, i.e. the fusion trees themselves, in a stable manner. Here, again, I think a string representation should again be valid. I don't remember having changed the fusion tree structure in any version change of the last few years. |
|
I did actually at some point start looking at trying to get some form of spec for this, similar to what Tenpy has but with the inclusion of non-abelian or categorical symmetries. I do think that storing all fusiontrees as strings might be slightly too inefficient for practical purposes, so I think my idea was to just use integers across the board. In other words, storing a list of subblocks for the data, and then either a matrix of integers or a 3d array of integers to hold all of the fusiontree data, where the first dimension is the legs within a tree, the second is either the subblock index or for product sectors the product sector index with then the last one the subblock index. Probably for convenience (and possible interoperability) it could be nice to store only uncoupled for abelian, add a field for inner and vertices only for the sectors that require them. This still requires choosing an integer for the charges and agreeing on clebsch Gordans but otherwise kind of fully fixes the data? Remind me if I forget, but I'll try and dig up that spec next week |
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.