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

Support Asymmetric Correlations #174

Closed
wants to merge 31 commits into from

Conversation

Lazersmoke
Copy link
Contributor

Out of equilibrium, correlations <A(t) B(0)> and <B(t) A(0)> are different; https://journals.aps.org/prl/pdf/10.1103/PhysRevLett.131.077101

This PR adds support for asking Sunny for these asymmetric correlations. In the LSWT case, this does nothing because the Sab tensor is explicitly symmetric there (for now).

In the classical case, this PR makes two changes:

  1. The correlations no longer "loop around" using the periodization of the fourier transform convolution. Previous to this PR, the computed correlation was a linear combination of <B(0) A(t)> + <B(0) A(t - T)> where T is the length of the sample trajectory, and t < T. Thus, the A -> B and B -> A correlations were mixed up. This PR resolves this by padding the B trajectory with zeros, performing the convolution, then trimming the padding (which now contains unphysical loop-around artifacts). This is not optimized yet
  2. Changes the observables interface by allowing e.g. both (Sx,Sy) and (Sy,Sx) correlations, and no longer assuming hermitian Sab anywhere. Additionally, a new mode :all_available is allowed, which is documented and comes with flavor text explaining the order of the correlations within the vector of correlations that is returned. This is mainly to allow users to ask for (Sx,Sy) and (Sy,Sx) correlations easily without requiring a custom intensity formula and without computing all 9 correlations. By default now, all 9 3x3 dipole-dipole correlations are computed, since they are generally different. Having an asymmetric Sab tensor means that you are out of equilibrium, and is one way to recognize that Langevin didn't do what you wanted

There is a test now which compares the new method of correlation calculation to an FFT-less reference computation.

@kbarros
Copy link
Member

kbarros commented Sep 29, 2023

Overall this looks good to me. There is a failing test on line 6 of test_correlations.jl due to I not being defined?

    sc = dynamical_correlations(sys; Δt = 0.1, ωmax = 10.0, nω=100, observables = [:A => I(2), :B => I(2)])

@Lazersmoke
Copy link
Contributor Author

Lets discuss at the meeting: Are these the correct correlations to be computing? I think that they are but I'm still in the process of verifying this myself. In particular, how are correlations at negative times to be treated (currently, see below, all correlations are at positive times)? This can be very relevant for glassy systems.

for t = 0:198
for tau = 0:198
q11[1+t] += As[1+(t+tau)] * As[1+(tau)]
q12[1+t] += As[1+(t+tau)] * Bs[1+(tau)]
q21[1+t] += Bs[1+(t+tau)] * As[1+(tau)]
q22[1+t] += Bs[1+(t+tau)] * Bs[1+(tau)]
end
end

@Lazersmoke
Copy link
Contributor Author

Ok, status update on this PR:

  • The interpretation of the classical correlation matrix changes in this PR. It is now one-sided (positive time, causal) correlations <A(t) B> for strictly t > 0. The old interpretation is recovered by symmetrizing. The dipole factor needs to updated to reflect this and hasn't been done yet. This semantic change should be documented and we should discuss whether LSWT can/should get the same treatment (i.e. it would store the residues and poles for unilateral laplace transform). I think it should be similarly modified--time-forward correlations are more fundamental
  • There is still that factor 2pi in LSWT which needs to be put somewhere/documented. Since the classical code is changing to allow for asymmetric correlations, the FFT factor is also changed, and I don't know how to verify these are compatible in the correct way

Those bolded points should be addressed before merging.

changes to BandStructure, which I need as hooks for susceptibility code, but which are also relevant here due to the 2pi factor change (which is conceptually about what are the semantics of BandStructure/"the residues")

  • There is now a Base.map instance for BandStructure. I want to export the BandStructure type because it's so user-friendly. Objections?
  • There is now an extremely simple, one line, (unexported) intensities_band_structure function, which does the same thing as intensities_bands but returns the result in terms of BandStructure instead of ad-hoc lists

Other stuff that's in this PR due to churn but is helpful and should stay:

  • There is now an unexported plot_band_intensities function, which does what the FeI2 tutorial does with the colored lines.
  • There are some commented-out stubs for finite-kT spin wave interface, which is not yet implemented but has interplay with the changes in this PR
  • SpinWaveTheory now stores the quantization basis for each site, at the same expense as storing the rotated onsite coupling matrix (this is a hook for my eigenmode viewer, but conceptually an eigensolver is kinda incomplete if it only returns "eigenvalues and not eigenvectors")

@Lazersmoke
Copy link
Contributor Author

This is ready to merge in my opinion!

@kbarros
Copy link
Member

kbarros commented Oct 24, 2023

If this works like we're hoping then we can get rid of process_trajectory kwarg, e.g., these docs.

- `process_trajectory`: Specifies a function that will be applied to the sample
    trajectory before correlation analysis. Current options are `:none` and
    `:symmetrize`. The latter will symmetrize the trajectory in time, which can
    be useful for removing Fourier artifacts that arise when calculating the
    correlations.

@kbarros
Copy link
Member

kbarros commented Oct 24, 2023

After the Zoom call, my understanding is that the normalization of LSWT intensities is exactly unchanged, but we need to verify the normalization of intensities from classical dynamics. Specifically, is there a test of the sum rule? Also, if the overall scaling factor is changing anywhere for intensities, we need to document this as a breaking change in versions.md

@ddahlbom
Copy link
Member

Thanks for making all these revisions. I'll spend some time with this tomorrow (run some tests, make sure I understand more of the details). This seems like a neat approach.

@Lazersmoke
Copy link
Contributor Author

Closing in favor of #217

@Lazersmoke Lazersmoke closed this Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants