Skip to content
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

Modify TagSet comparison (==) ? #314

Open
emstoudenmire opened this issue Dec 21, 2019 · 4 comments
Open

Modify TagSet comparison (==) ? #314

emstoudenmire opened this issue Dec 21, 2019 · 4 comments
Labels

Comments

@emstoudenmire
Copy link
Contributor

TagSets can easily end up with negative prime level versus zero prime level depending on how they are constructed. For example, the default TagSet constructor makes the internal prime level equal to -1. But constructing a TagSet from a string passed through an Args object results in a TagSet with prime level 0.

Even though both TagSets are interpreted as both having prime level zero in terms of how they affect indices constructed from them, the operator==(TagSet,TagSet) comparison function treats these tag sets as different.

Because of this, the check on line 85 of itensor/svd.cc is not succeeding at catching some cases where litagset and ritagset are effectively the same in that they result in the Indices uL and vL being exactly equal.

Should TagSet comparison be more relaxed and treat an internal prime level of -1 as the same as zero? Or should the primeLevel(TagSet) function itself return 0 when the internal number is -1?

@mtfishman
Copy link
Member

Internally, a prime level of -1 is interpreted as the state that no prime level was input, and certain TagSet functions act differently if a prime level does or does not exist in the TagSet, so this would likely cause problems/inconsistencies elsewhere. For example, the addTags functions requires that there is no prime level in the TagSet getting added, since it doesn't makes sense to add a prime level through the addTags function, so it checks if the TagSet has a prime level of -1. I think changing the definition of equality would be a bit confusing.

I think for the case here that caused the bug, it may be better to go back to the old way the svd function was designed, where the two new internal indices had different id's. That would avoid the issue entirely, since it would then be ok for the tags to be the same. I think it is a better design anyway, since the QN version of the svd introduces indices with different id's (which it has to, since the flux rules of the svd dictate that the indices in general will have different QNs).

If other issues like this pop up that don't have nice solutions, we may consider your suggestion to separate the prime level from the TagSet. Unfortunately it is embedded in the notation right now, so it would be a breaking change (but possibly we could phase it out with deprecation warnings).

@emstoudenmire
Copy link
Contributor Author

Thanks for the helpful breakdown of the issue. Alexios contacted me yesterday to say he is running into a different version of the same bug now, even after my previous fix.

I think having different id's on the singular value tensor makes a lot of sense, especially if it happens anyway in the QN version. I think it's good for most things to be as consistent as possible (within reason) between QN and dense tensor behavior, just to avoid surprises.

Yes let's keep an eye on these kinds of issues and consider the more extreme move of separating primes and tags as you say only if we find some cases which don't have as nice of a solution as this one.

@emstoudenmire
Copy link
Contributor Author

I meant to say I'm happy to do this suggested fix, since I was already working on the issue previously - after that I'll mark this issue as closed.

@mtfishman
Copy link
Member

I think the new issue Alexios is seeing is not an issue directly with the position function (see the email I sent him).

Thanks for handling the change to the svd function, it should be a very small fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants