Open acroscarrillo opened 1 month ago
Thanks for bringing this up!
For better or worse, this is expected (and documented (maybe poorly)) behavior.
entanglement_entropy(::Ket)
gives what you expectentanglement_entropy(::AbstractOperator)
gives the operator space entanglement entropyThis seems to be a reasonable description of "operator space entanglement entropy": https://arxiv.org/pdf/2201.05099
Historically the developers of this package have decided on this set of semantics. There are good reasons to do so, and I am reluctant to consider a breaking release that changes the semantics. Either way, this can be more vividly documented (or potentially deprecated in favor of separate entropy_ent
and entropy_opent
).
I will change the title of this issue and keep it open to keep track of this.
@Krastanov Thank you for chipping in so quickly! In my humble opinion this is beyond semantics, a state either as a ket $|\psi\rangle$ or a density matrix $|\psi\rangle \langle \psi|$ must give the same results. I won't go into the details of "entanglement entropy" vs "operator space entanglement entropy" but at the very least, a deprecation in favour of separate entropy_ent
and entropy_opent
seems like an excellent idea! May I suggest instead of entropy_opent
the term entropy_op_ent
? I can't help but to read "open-t" as opposed to "op-ent" !
@david-pl , @amilsted , how do you feel about a potential deprecation and split here? Something like:
entanglement_entropy(::Ket)
works the same, without a warning (adding a deprecation warning a few years from now)entanglement_entropy(::Operator)
works the same, with a deprecation warningentropy_ent
that works on ::Ket
and ::Operator
and returns entropy_vn(ptrace(...))
entropy_op_ent(::Operator)
that returns operator-space entanglement entropyWe can keep these deprecation warnings for a few years before we act on them.
Personally, I do feel the current state, while perfectly consistent and rigorous, would lead to similar confusing experience in the future.
I'm open to this change, especially if we deprecate the current method. Admittedly, I didn't use this functionality too much myself. However, I see how this can be confusing and that can this will likely trip up any first time user. There's quite a few parts in QO.jl that could do with better usability and I think this is one of them.
@acroscarrillo would you be open to put this split in a PR?
@david-pl I'm more than happy to help but I'm fairly new to collaborative coding. Could you guide me a little bit? I'm okay with the code writing part, it is a simple change
Thanks, Alejandro! Here is a typical workflow (but not necessarily the only possible workflow). You might already know much of this, but I am typing it out for completeness:
SOME_FOLDER
with its own SOME_FOLDER/Project.toml
SOME_FOLDER/QuantumOpticsBase.jl
SOME_FOLDER
start a julia process julia --project=.
] dev ./QuantumOpticsBase.jl
and ] add
any other packages you care about. dev
ensures that any changes to this local folder get reflected next time you import the local version of the library.using Revise
before using QuantumOpticsBase
, so that you do not need to restart the julia process as you modify things.git checkout -b better_ententropy_convention
git add .
to add all these changes and then git commit -m "SOME SHORT DESCRIPTION OF THE CHANGES"
git status
between each git command to make sure that the state of your repo is what you expectA few notes:
SOME_FOLDER
as its own window in VSCode. VSCode editing workflow is really meant to have one-folder to one-project to one-window correspondance.@Krastanov thanks for your guidance! I think I've done all the steps and got stuck at the last, I get:
2024-06-21 11:37:48.332 [info] remote: Permission to qojulia/QuantumOpticsBase.jl.git denied to acroscarrillo.
fatal: unable to access 'https://github.com/qojulia/QuantumOpticsBase.jl.git/': The requested URL returned error: 403
Im doing it using the official VS code "GitHub Pull Requests" extension:
I believe the issues might be a step before creating the PR.
Looking at this screen shot, it is suggesting that you create a pull request for the merging of qojulia/QuantumOpticsBase.jl/entanglement_entropy
into the official repository. But qojulia/QuantumOpticsBase.jl
is the official repository to which you do not have write access to have created an entanglement_entropy
branch. And indeed if you look at the list of branches on the official repository qojulia/QuantumOpticsBase.jl
there is no entanglement_entropy
branch.
Double check that your git push
worked (a repeated push should just tell you there is nothing new to push).
Looking at your github profile I do not see a fork of QuantumOpticsBase.jl (unless you have made a private fork). You need your own fork to which you can write your branches for review before merging into the official repository. I suspect you have made a local clone of the official repository instead of making a clone of your own fork.
For instances, here is what I see if I ask my local repository on my computer to tell me what remote repositories it knows of:
origin
is my fork and upstream
is the official repository. Whenever I make new changes, I make a branch on my computer, push it to origin
which is my fork and use the github website to create a pull request to upstream
. It is a bit convoluted like this because git was never really designed from the get-go to be used through a website. I think github has their own command line tool that simplifies some of these tasks, but I have never used it.
Edit (by @Krastanov): This is expected behavior. See first comment below for possible ways forward.
Original post below:
There seems to be a bug in the entanglement entropy calculation, it seems like it might be a factor of 2 off?
Here's the code to reproduce the error:
I create the following state and check is normalised:
Now I calculate the entanglement between the A={1,2} and B={3} as follows:
However, this is wrong. I have done the calculation analytically (two methods, SVD and partial tracing) and it should be $-\frac{2}{3}\log(\frac{2}{3})-\frac{1}{3}\log(\frac{1}{3}) \approx 0.6365471166931516$, which could look like $1.2730283365896256 / 2$ but not exactly?
It's interesting cause your ptrace agrees with my calculations since
and by inspection this spectrum does indeed agree with my calculations. I am a bit puzzled cause in the source code you guys have
so it should be doing the same provided the spectrum is the same, which it should be since it is being passed the same density matrix.
To check I am not crazy:
I guess it could be that I am miss using the
partition
argument, I didn't find the documentation very clear.