SunnySuite / Sunny.jl

Spin dynamics and generalization to SU(N) coherent states
Other
86 stars 19 forks source link

Lswt general observables #156

Closed Lazersmoke closed 1 year ago

Lazersmoke commented 1 year ago

This adds support for arbitrary user-specified observables (NxN matrix) to LSWT SU(N) mode by factoring out the existing custom-observable support from SampledCorrelations into a new struct ObservableInfo, which then is shared between LSWT and classical modes. Note that LSWT dipole mode does not support NxN matrix observables; it only supports the default Sx,Sy,Sz observables.

In addition, this change considerably simplifies SampledCorrelations because it no longer needs to worry about the observables as much

kbarros commented 1 year ago

This PR builds on top of https://github.com/SunnySuite/Sunny.jl/pull/125, so that one would need to be merged first.

It looks like the new thing here is the "Add ObservableInfo" commit.

Separately, the "Upgrade developer-facing show instances" commit no longer exists on main, and now lives separate branch, detailed-show. We can revisit which of those features to merge into main at a later time.

Lazersmoke commented 1 year ago

Yep, for some reason the merge is much messier when targeting #125 directly.

Edit: and the reason for building off of #125 is that it was easier to build off the existing LSWT refactor there.

ddahlbom commented 1 year ago

I'll take a look a little later today.

On Mon, Sep 4, 2023, 15:42 Sam Quinn @.***> wrote:

@.**** commented on this pull request.

In test/test_binning.jl https://github.com/SunnySuite/Sunny.jl/pull/156#discussion_r1315177170:

@@ -94,7 +94,7 @@ is_flat[m + (k-1) * 6,1,1,l] = is[k,1,1,l][m] end

  • @test isapprox(is_flat,is_golden;atol = 1e-12)
  • @test_broken isapprox(is_flat,is_golden;atol = 1e-12)

Note that I have disabled these tests as "broken" -- something strange is going on because the classical :full appears broken while the LSWT :full is working.

— Reply to this email directly, view it on GitHub https://github.com/SunnySuite/Sunny.jl/pull/156#pullrequestreview-1609915544, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIERZRZJXYZGI77O6DYUSATXYYVJXANCNFSM6AAAAAA4J2Y5UA . You are receiving this because your review was requested.Message ID: @.***>

Lazersmoke commented 1 year ago

Thanks David, good catch! Glad it is just the test that's broken 😅, will update it soon

On Mon, Sep 4, 2023, 18:28 David A. Dahlbom @.***> wrote:

@.**** commented on this pull request.

In test/test_binning.jl https://github.com/SunnySuite/Sunny.jl/pull/156#discussion_r1315218428:

@@ -94,7 +94,7 @@ is_flat[m + (k-1) * 6,1,1,l] = is[k,1,1,l][m] end

  • @test isapprox(is_flat,is_golden;atol = 1e-12)
  • @test_broken isapprox(is_flat,is_golden;atol = 1e-12)

I think you just need to update the test. After your refactor, is[k,1,1,l][m] is indexing into a 3x3 array rather than a length-6 SVector.

The following quick hack fixes the test.

is_flat = zeros(ComplexF64,size(is_golden)) midcs = collect(keys(sc.observables.correlations)) |> reverse for k = 1:4, l = 1:3, m = 1:6 is_flat[m + (k-1) * 6,1,1,l] = is[k,1,1,l][midcs[m]] end isapprox(is_flat,is_golden;atol = 1e-12) # Now passes

— Reply to this email directly, view it on GitHub https://github.com/SunnySuite/Sunny.jl/pull/156#discussion_r1315218428, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAW3RJAZQ6XMEC6RRV5JC5LXYZIX3ANCNFSM6AAAAAA4J2Y5UA . You are receiving this because you authored the thread.Message ID: @.***>

ddahlbom commented 1 year ago

Looked at this again a bit more closely this morning. There are a few things that didn't sink when I was going through the diff yesterday.

This all looks exactly like what we want and should not interfere with future refactors (since, for example, the observables in the SWT at this point keep their own rotated spin operators -- this will allow us to remove the spin and quadrupole operators from the SWT struct without issue).

Lazersmoke commented 1 year ago

Hmm so I think we still need the dipole and quadrupole operators in the SWT struct for constructing the Hamiltonian. Unless the future refactor constructs it without needing those? It's true that they are not needed for the reading out of values at the end

On Tue, Sep 5, 2023, 10:46 David A. Dahlbom @.***> wrote:

Looked at this again a bit more closely this morning. There are a few things that didn't sink when I was going through the diff yesterday.

This all looks exactly like what we want and should not interfere with future refactors (since, for example, the observables in the SWT at this point keep their own rotated spin operators -- this will allow us to remove the spin and quadrupole operators from the SWT struct without issue).

— Reply to this email directly, view it on GitHub https://github.com/SunnySuite/Sunny.jl/pull/156#issuecomment-1706764139, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAW3RJF2GYJGKXQXWBBTDXTXY43NJANCNFSM6AAAAAA4J2Y5UA . You are receiving this because you authored the thread.Message ID: @.***>

ddahlbom commented 1 year ago

Right. The interactions in the Hamiltonian will not be constructed in terms of relations between spin operators, quadrupoles, etc., but instead in terms of a set of bond-dependent operators that render the interactions maximally sparse. Kipton worked it out, wrote a Lyx note about it. I think it's in the repo. Familiar observables will only appear when initially specifying interactions and when querying intensities.

Lazersmoke commented 1 year ago

OK this is almost ready except for one detail: The OnsiteCoupling has changed to a union type at some point, so this needs to be made consistent. This branch also includes detailed-show, which also needs to be updated (it currently prints both the matrix and the stevens expansion, which doesn't make sense with the union type)

Side note about this: The stevens expansion is always traceless!!! So it seems to me like a bad idea to ever just use only the stevens expansion

kbarros commented 1 year ago

This branch also includes detailed-show, which also needs to be updated (it currently prints both the matrix and the stevens expansion, which doesn't make sense with the union type)

Could you please create a commit that lives on top of main and just has the Observables updates? (it's not clear what will happen to detailed-show, let's not stack PRs.)

The stevens expansion is always traceless!!! So it seems to me like a bad idea to ever just use only the stevens expansion

The matrix representation is also shifted to be traceless. The current design disallows an onsite coupling to make a constant shift to the energy.

kbarros commented 1 year ago

The stevens expansion is always traceless!!! So it seems to me like a bad idea to ever just use only the stevens expansion

The matrix representation is also shifted to be traceless. The current design disallows an onsite coupling to make a constant shift to the energy.

This is fixed in https://github.com/SunnySuite/Sunny.jl/commit/db7eab5037d2dd5055bdf5f068fba4c8d474615d. Now the matrix trace is retained, which corresponds to keeping the $\mathcal{O}_{0,0}$ term of the Stevens expansion.

This change allows anisotropy operators to introduce a constant shift in the energy, but otherwise, shouldn't have any effect on physical observables. In SU(N) mode we never cared about this constant energy shift before, but now that the user can set "large S" anisotropies, the change becomes important for reducing confusion.

kbarros commented 1 year ago

Closing in favor of https://github.com/SunnySuite/Sunny.jl/pull/167 which squashes the commits and rebases them on main.