[WIP] [ITensors] Reorganize test exports #1410
Conversation
| :hastags, | ||
| :id, | ||
| :ind, | ||
| :index_id_rng, # remove export | ||
| :inds, | ||
| :insertblock!, # remove export | ||
| :isactive, # remove export | ||
| :isfermionic, # remove export | ||
| :ishermitian, # remove export | ||
| :isindequal, # remove export | ||
| :itensor, | ||
| :lq, # remove export | ||
| :mapprime, | ||
| :mapprime!, # remove export (not defined?) | ||
| :matmul, # deprecate | ||
| :matrix, # deprecate | ||
| :modulus, # remove export | ||
| :mul!, # remove export | ||
| :nblocks, | ||
| :nnz, | ||
| :nnzblocks, | ||
| :noncommonind, | ||
| :noncommoninds, | ||
| :noprime, | ||
| :noprime!, # deprecate | ||
| :norm, # remove export | ||
| :normalize, # remove export | ||
| :normalize!, # remove export | ||
| :not, # remove export | ||
| :nullspace, | ||
| :nzblock, | ||
| :nzblocks, | ||
| :onehot, | ||
| :order, | ||
| :permute, | ||
| :plev, | ||
| :polar, | ||
| :pop, | ||
| :popfirst, | ||
| :prime, | ||
| :prime!, | ||
| :push, | ||
| :pushfirst, | ||
| :ql, | ||
| :qn, | ||
| :qr, | ||
| :randn!, # remove export | ||
| :readcpp, | ||
| :removeqn, | ||
| :removeqns, | ||
| :removetags, | ||
| :removetags!, # deprecate | ||
| :replaceind, | ||
| :replaceind!, | ||
| :replaceindex!, | ||
| :replaceinds, | ||
| :replaceinds!, | ||
| :replacetags, | ||
| :replacetags!, | ||
| :reset_warn_order!, | ||
| :rmul!, | ||
| :rq, | ||
| :scalar, | ||
| :scale!, | ||
| :set_warn_order!, | ||
| :setdir, | ||
| :setelt, | ||
| :setindex, | ||
| :setprime, | ||
| :setprime!, | ||
| :setspace, | ||
| :settags, | ||
| :settags!, | ||
| :sim, | ||
| :sim!, | ||
| :space, | ||
| :storage, | ||
| :store, | ||
| :svd, | ||
| :swapind, | ||
| :swapinds, | ||
| :swapinds!, | ||
| :swapprime, | ||
| :swapprime!, | ||
| :swaptags, | ||
| :swaptags!, | ||
| :tags, | ||
| :transpose, | ||
| :unionind, | ||
| :unioninds, | ||
| :uniqueind, | ||
| :uniqueindex, | ||
| :uniqueinds, | ||
| :use_combine_contract, | ||
| :use_debug_checks, | ||
| :vector, | ||
| :δ, | ||
| :⊕, | ||
| :⊙, | ||
| ] |
There was a problem hiding this comment.
@emstoudenmire please go through this list and see if there is anything we can argue could get moved to the ITensorMPS.jl export list, which would mean they would get removed from the ITensors.jl export list once we move ITensors.ITensorMPS to ITensorMPS.jl.
Many of them should be, or already are, deprecated, and I started to comment about those, but that is a separate issue. We should still try to categorize them by ones that are more associated with MPS operations vs. more general ITensor operations. Note that we may remove some things from the ITensors.jl export list, like OpSum, which we will keep in ITensors.jl since other packages like ITensorNetworks.jl will be using them.
There was a problem hiding this comment.
Good question... Looking over the above list a few times, I did not see any that jumped out at me as being "MPS only". Some like replacetags certain have notable MPS/MPO overloads, but also make sense purely at the ITensor level. Were there any specifically you were still undecided about in terms of which package they should get exported from?
I do agree that reducing this list and carrying through any deprecations or planned deprecations is a great idea, with the aim of reducing the exports.
There was a problem hiding this comment.
The main ones I was most uncertain about were the ones that were not obviously ITensor or MPS functionality, i.e. functions that you brought up below like state, siteind[s], op, etc. which can be used with either ITensor operations or MPS/MPO operations but are more often used in association with MPS algorithms in practice. I'm biasing towards moving them to ITensorMPS (at least their exports, for the time being their implementation will live in ITensors.jl) but it is helpful to get a second pair of eyes on that decision as a sanity check. Also it is a long list so I wanted to double check I wasn't making an obvious mistake somewhere.
mtfishman
added
the
ITensors
| :uniqueindex, | ||
| :uniqueinds, | ||
| :use_combine_contract, | ||
| :use_debug_checks, | ||
| :val, |
There was a problem hiding this comment.
Similarly, I would say the above list of ITensorMPS exported names all look to be correctly MPS related, and not needed for ITensors.jl alone, with maybe these exceptions:
statesiteindsiteindsmaybe also ifsiteindgoes back into ITensors.jl?- similarly
opand related functions
Ah but I see, you have moved these into a lib/ with an extension (I saw the PR about that but only just remembered). So then is the idea of moving these exports to ITensorMPS more about whether they are exported by default (as ITensorMPS would) versus making them optional (as ITensors.jl now would)?
There was a problem hiding this comment.
Yes, that's a good summary. Glad to see you were able to determine the logic behind these decisions without an explanation, that's a good sanity check!
Yes, the idea would be that we could provide some alternative for loading functionality like state, siteind[s], op, etc. instead of exporting them by default from ITensors.jl. The initial suggestion will be to tell users to load ITensorMPS.jl (which is not much worse than the current situation anyway, in terms of burden of how much code gets loaded/compiled/etc.). In the future the goal will be to split off that functionality into separate packages that are either independent of or built on top of ITensors.jl.
For example, I could imagine something like:
LocalHilbertSpaces.jl: Definitions likeSiteType,siteind,siteinds, etc. (maybe with one layer that is independent of ITensor that just builds spaces as axes/graded axes and another that buildsIndexobjects).QuantumStates.jl: Local state definitions (up, down, etc.) and things likeStateName, independent of ITensor (say outputs dense or sparse vector representions of states or functions for setting elements independent of the particular data structures).QuantumOperators.jl: Local operators definitions (Sx, Sy, etc.) and things likeOpName, independent of ITensor (say outputs dense or sparse matrix representions of operators or functions for setting elements independent of the particular data structures). Maybe combine withQuantumStates.jl.ITensorStates.jl/ITensorOperators.jl: Compatiblity layer gluing togetherQuantumStates.jl/QuantumOperators.jlwithITensors.jlthat has the task of converting state and operator definitions into ITensor representations.
Obviously the particular modular structure and names will need to be discussed, I'm just giving a brief outline of a potential design.
So we could picture that these packages, along with LazyApply and Ops, sit between ITensors.jl and ITensorMPS.jl/ITensorNetworks.jl. But for the time being while we work out all of those details we can give users the instructions that they should load ITensors and ITensorMPS if they want customizable Hilbert space, state, and operator construction functionality. The only downside of that is that users that want Hilbert space, state, and operator definitions may find it strange that they have to load a full MPS package to get that, but that is already what we make them do with ITensors.jl so I don't think it is too bad. The benefit for us is that it will give us a lot more flexibility to break apart ITensors.jl into smaller composable pieces in the medium to long run while imposing fewer breaking changes onto users during that process.
|
The plan here is that once the initial version of ITensorMPS.jl is registered (JuliaRegistries/General#106277), I plan to use @test issetequal(names(ITensors), ITENSORS_EXPORTED_NAMES)
@test issetequal(names(ITensors), [ITENSORBASE_EXPORTED_NAMES; names(ITensorMPS)]), i.e. test that the planned future exports of |
mtfishman
changed the title
…/ITensors.jl into ITensors_reorganize_test_exports
|
ITensorMPS.jl is registered and exports the list suggested in this PR and also has a test for that list: https://github.com/ITensor/ITensorMPS.jl/blob/v0.1.2/test/utils/TestITensorMPSExportedNames.jl so I'll close this. |
Reorganize test exports into two lists, one which lists names that will be exported by
ITensorMPSand the other will remain exported byITensors.This list is also reflected in the exports and tests of the new
ITensorMPS.jlrepository (ITensor/ITensorMPS.jl#1).It may be superfluous to make this change here but I found it was helpful to have a PR like this to assess which exports we should move over to
ITensorMPS.jl.@emstoudenmire