SunnySuite / Sunny.jl

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

`recipvecs` not set properly in LSWT #109

Closed Lazersmoke closed 1 year ago

Lazersmoke commented 1 year ago

https://github.com/SunnySuite/Sunny.jl/blob/188846860b8fb6a447f8400f854311c340003b68/src/SpinWaveTheory/SpinWaveTheory.jl#L160-L164

cc: @ddahlbom , I think this is not doing the correct thing. Various parts of the code rely on the swt.recipvecs_chem to be accurate (e.g. connected_path and the binning code for LSWT), but here it is sometimes set to the identity matrix.

ddahlbom commented 1 year ago

Going to ping @Hao-Phys in case he has some insights. I don't think I ever modified these lines of the code other than to add the comments (at least I don't think I ever changed in semantically). I just added some notes about what I wanted to check or revise if I were to dig into it.

My basic model was that, at least internally, requests for intensities should always be given in RLUs (in terms of the originally, un-reshaped lattice). In my experience Hao's code has always worked with this way, but perhaps some cases escaped my attention.

The intention for connect_path was that it would work exclusively with RLUs, and should work directly with the original system (not the SWT object itself).

Happy to dig into this further when I get back next week if it would be helpful.

kbarros commented 1 year ago

I concur with David's point that Sunny tries hard to always convert back to RLU fof the original, unreshaped crystal, and it's easy to make a mistake in these conversions. I'll also take a look tomorrow.

ddahlbom commented 1 year ago

(I think it was a mistake for me to add a connected_path method that takes an SWT. Probably there should only be a version that takes a System.)

kbarros commented 1 year ago

I think this will be resolved by https://github.com/SunnySuite/Sunny.jl/pull/116.