JuliaAttic / QuBase.jl

A foundational library for quantum mechanics in Julia
Other
43 stars 6 forks source link

Dot product for QuArray's. #36

Closed amitjamadagni closed 9 years ago

amitjamadagni commented 9 years ago

I was going through the material on Krylov propagator, as it requires the dot of QuArray's I have added this, I am not really sure if we require this , any comments on this would be helpful.

acroy commented 9 years ago

+1 for dot. We should also have this for QuMatrix. Related JuliaLang/julia#11064.

acroy commented 9 years ago

The Travis error for 0.4 seems unrelated, but we should have a closer look.

amitjamadagni commented 9 years ago

@acroy @jrevels I have made an attempt at dot (considering dual vectors as well) and trace, added tests for the same. A review would be helpful.

acroy commented 9 years ago

Apart from the comment on the tests, I think this looks good.

amitjamadagni commented 9 years ago

@acroy edits done. If this is fine, I will squash the commits ?

amitjamadagni commented 9 years ago

@acroy done!

acroy commented 9 years ago

Good. Please squash the commits and then we can merge I would say.

acroy commented 9 years ago

Sorry. I just noticed that you squashed already :-)

amitjamadagni commented 9 years ago

@acroy can we merge this ?

acroy commented 9 years ago

Basically yes. I was just thinking if we should generalize dot to allow for two AbstractQuVectors (and giving an error in the mixed case)?

amitjamadagni commented 9 years ago

@acroy I have tried creating a common dot for AbstractQuVector.

acroy commented 9 years ago

I see, but what I meant was the following

dot{B<:OrthonormalBasis}(vec1::AbstractQuVector{B}, vec2::AbstractQuVector{B}) = ...
dot{B<:OrthonormalBasis}(vec1::DualVector{B}, vec2::DualVector{B}) = ...
dot{B<:OrthonormalBasis}(vec1::DualVector{B}, vec2::AbstractQuVector{B}) = error("Inner product of a dual vector and a vector is not supported.")
dot{B<:OrthonormalBasis}(vec1::AbstractQuVector{B}, vec2::DualVector{B}) = error("Inner product of a dual vector and a vector is not supported.")

and maybe similar for AbstractQuMatrix.

amitjamadagni commented 9 years ago

@acroy the latest commit covers the cases, please do let me know if I am missing something here.

acroy commented 9 years ago

You are checking if the element types of the two vectors are the same, while I simply want to prevent a DualVector and an AbstractQuVector as parameters.

amitjamadagni commented 9 years ago

Sorry my bad I understand what I did here. So what would be the best way out, the initial version or the latest version ?

acroy commented 9 years ago

Either you revert it to your original version and we sort it out later or you just take the code I posted above and replace the ... with the code you provided originally.

amitjamadagni commented 9 years ago

Would be good if we compare the types of vectors i.e., typeof(vec1) == typeof(vec2) which would then generalize to comparing QuArray and CTranspose ?

acroy commented 9 years ago

You don't need to compare the types. dot{B<:OrthonormalBasis}(vec1::AbstractQuVector{B}, vec2::AbstractQuVector{B}) actually applies to all vectors (including dual vectors). Now we have to provide more specific versions to exclude the cases with mixed parameters. All the work will be done by the compiler.

amitjamadagni commented 9 years ago

@acroy a review would be helpful.

acroy commented 9 years ago

@amitjamadagni : Could you please squash and rebase?

amitjamadagni commented 9 years ago

@acroy done !

acroy commented 9 years ago

@amitjamadagni : Thanks!