-
Notifications
You must be signed in to change notification settings - Fork 6
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
QuCoeffs: A more controlled approach to QuArray coefficient containers #10
Conversation
Thanks for doing this! It certainly looks promising. I will also try to have a closer look (time is a bit constrained these days). |
# QuCoeffs # | ||
############ | ||
abstract ConjBool{Conj} | ||
abstract TranBool{Tran} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need new types here? Aren't bools sufficient?
Allowing for conjugation and transposition separately is certainly most general, but do we have a use case for this? I thought that indicating "duality" (whatever that means in the specific case) would be enough (and maybe easier to grok for implementing inner products and such).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bool wrapper types are there for type stability purposes. Otherwise, the only type stable way to construct an instance of QuCoeffs would be to manually declare them as parameters (which is obviously annoying since you'd have to manually declare the other parameters as well) or have the constructor always set those parameters to false (which is limiting/less flexible).
As for only indicating duality - that's how I started, but then I ran into some of the problems discussed in JuliaLang/julia#6837. Mainly, figuring out which operations we want to result in copies and which we want to result in views. If we just had a duality indicator instead of separate conjugate and transposition indicators, then the behavior gets wonky/unintuitive - a transpose would cause a copy, a conjugation would cause a copy, but the ctranspose would cause a view. Thus, doing conj(transpose(arr))
would be a double copy operation, while ctranspose(arr)
would be a no-copy operation, which seems ugly/confusing to me.
I'm of the opinion that it makes the most sense for all three of the above operations to "agree" to return a view, or a copy. I obviously went with the view here, and tried to reduce the inefficiencies associated with computing conjugation on the fly by having it be a no-op for non-Complex
types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification. I should probably have a look into JuliaLang/julia#6837 again to be on the same page.
a transpose would cause a copy, a conjugation would cause a copy, but the ctranspose would cause a view. Thus, doing conj(transpose(arr)) would be a double copy operation, while ctranspose(arr) would be a no-copy operation, which seems ugly/confusing to me.
Maybe this would indeed be too confusing. I was just thinking that we mainly want v'=dual(v)
which typically just happens to be ctranspose(v)=conj(transpose(v))
(i.e. all three should return a copy, but dual
would return a view).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just thinking that we mainly want v'=dual(v) which typically just happens to be ctranspose(v)=conj(transpose(v)) (i.e. all three should return a copy, but dual would return a view).
Done. The code is definitely way cleaner and we avoid the view/copy mess, nice!
Disambiguating the dual
and ctranspose
functions will probably be a point of confusion for users, but hopefully nothing that thorough, explicit docs won't be able to clarify.
This certainly ooks interesting. Tensors from TensorToolbox.jl are still more general in that every single tensor index can be associated to a vector space or its dual (contravariant and covariant), or even to its conjugated or conjugated dual in the case of non-Hilbert spaces. This is not necessary to represent kets, bras and operators, but is necessary for e.g. representing states as tensor networks. Unfortunately I am having much less time than I thought and hoped to spend on this, so I have not really worked on this for a while now and it doesn't seem I'll have any time throughout January either. Maybe I should just try to get more people involved, but this also requires writing some basic documentation. |
@Jutho I have friends working closely in tensor networks. I am trying to get more people on board on our future directions. I would appreciate if you can provide a minimum documentation and some examples on your package before the SQuInT meeting (Feb 19-21). Some of our volunteers (most are not Julia experts) will have meetings there to think about if anyone would like to do something over the coming months. Hopefully it helps. |
|
||
setindex!(cv::CoeffsVector, x, y) = setindex!(cv, apply_conj(x, cv), y) | ||
setindex!(cv::CoeffsMatrix, x, y, z) = setindex!(cv, apply_conj(x, cv), y, z) | ||
setindex!(cv::TranMatrix, x, y, z) = setindex!(cv, apply_conj(x, cv), z, y) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of the advantages of not subtyping QuCoeffs to AbstractArray is that we can more easily enforce our own indexing scheme (and also are freed from having to make a bunch of functions to resolve ambiguities). I didn't implement indexing for N>2. @Jutho: You said TensorToolbox covers a more general dual indexing scheme for arbitrary N, yeah? Do you think it would make sense to focus our efforts with QuCoeffs on N<=2 case until TensorToolbox reaches a suitable state? I wouldn't want to repeat work :)
I would also be down to contribute to TensorToolbox if there was a little bit of documentation to help new contributors get going; no pressure, though. I, myself, won't have time to focus on anything besides QuBase/QuDirac until March (after SQuINT) anyway.
I did have another thought: Despite the obvious performance tradeoffs, one of the main reasons lazy transposition is so heavily discussed in Base right now is that it removes the conflation between row vectors and matrices, by allowing 1-D array transposition to return a 1-D array. We actually already have a way of disambiguating row vectors and matrices for QuArrays, though - the basis information (we set With this in mind, we could actually have eager duality for QuCoeffs, keeping track of conjugation/transposition in the type parameters (like the original incarnation of this PR). This would obviate the need for a special Once v0.4 releases, we could move to whatever lazy behaviors get decided on by that time. The counter-argument to the above is that we really desire the performance tradeoffs that come with lazy transposition, which I would understand, but would require quite a bit of work that might be better spent on the transposition issues in Base. Thoughts? I'm going to draft up a competing PR which implements what I'm talking about above, and we can decide between the two approaches. |
@jrevels I think we should let performance and intuition talk. Better to have something to compare in comprehensive tests before making the final decision, which should become an important criterion for judging. Maybe you can branch out those different ideas on the repo. All sounds interesting! BTW, regarding naming those types, I think it would be good to stick to the nomenclature that has been widely adopted in quantum mechanics textbooks. Certainly, we possibly will have to invent some new inner types for translating data inside Julia whenever it is necessary. I start wondering: are those names ( |
@i2000s That's why the QuCoeffs type is not exported. It is not intended to be manipulated by an end user.
If you have concrete suggestions for simplifying our type hierarchy, I'm all ears. Edit: And the most helpful form for these suggestions would be in a PR, if you have the time. |
@jrevels Hah, you did not export those things. Obviously, I have lagged off for a while. Sorry for my ignorance. |
This PR presents a new wrapper type,
QuCoeffs
, which gives us better control over the behavior of QuArrays' coefficient containers.Specifically, this is an attempt at providing a solution to #9.
Major changes:
QuCoeffs
type. This type provides a wrapper for lazy conjugation/transposition on arbitrary coefficient containers; I tried to incorporate some of the ideas from WIP:Add Transpose immutable JuliaLang/julia#6837. I specifically focused on getting the implementation to make sense for vectors and matrices (nothing N>2). Having QuCoeffs also helps separate some of the logical concerns that arise for QuArrays. Now, we can think of our type structure the following way:I'm really interested in getting @Jutho's eyes on this - my secret hope is that we could use TensorToolbox.jl to extend the functionality of (or maybe replace) QuCoeffs in the future.