Skip to content

Conversation

@alexhroom
Copy link
Collaborator

@alexhroom alexhroom commented Oct 8, 2025

Fixes #76

This PR also tidies up the code to increase reusability by moving lots of reused functionality into a utils.rs file.

@alexhroom
Copy link
Collaborator Author

conversation about outputs 8th dec: we want to do away with the neutron function and instead have calc_spinwave always return energies and sqw, optionally also returning sab if desired

@alexhroom alexhroom marked this pull request as ready for review December 9, 2025 12:46
@alexhroom alexhroom requested a review from mducle December 9, 2025 12:46
Copy link
Member

@mducle mducle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok... I've just got a few minor points, but I'm not sure why the tests are failing...

@alexhroom
Copy link
Collaborator Author

@mducle tests still fail but are a lot better after the changes to add a zero energy tolerance... and i've updated the examples to display the energies and sqw as two subplots on top of each other so it's a bit easier to see where the weird blow-ups are happening (it seems the only remaining massive one that won't fix itself is at small eigenvalues for the kagome example in Python)

@mducle
Copy link
Member

mducle commented Dec 17, 2025

Thanks, I'll have a look at it later and will get back to you. Enjoy your break!

Copy link
Member

@mducle mducle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to see how to fix the test discrepancies between Python and Rust calculations and found two parts where the rust calculations differ from the Python; however I still can't get the tests to pass - there is still some bug somewhere.

Looking at the values returned by Python and Rust codes I think that there is definitely some inconsistencies between the code and it's not just a matter of the intensities blowing up near regions where the eigenvalues goes to zero.

Also, both Python and Rust calculated intensities is different from the Matlab - mostly the differences are small but at some Q points they are significant so I'm still hunting this issue down...

Copy link
Member

@mducle mducle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, one last set of changes!

There two main issues causing the disagreement between Python and Rust (and hence test failures):

  1. Degenerate eigenvalues: the eigenvectors for degenerate eigenvalues are not guaranteed to be unique or orthogonal by the eigendecomposition being computed in either Python or Rust - they can be any linear combination of each other and still satisfy the maths. Now, physically this is because we cannot determine separately the intensities of each degenerate mode - we can only measure their sum; And I think if we sum the intensities of degenerate modes we would find that the Rust and Python codes agree. (The sw_omegasum function in Matlab SpinW does this). However, for our tests we are just comparing the raw values and this will be different depending on the numerics of numpy and faer.

  2. There is something wrong with the lblt decomposition for the kagome_ferromagnet example... I can get slightly better agreement for some Q-points by doing an addition permutation of the points but I can only get the test to pass using the ldlt decomposition (which is not guaranteed to succeed)...

Anyway, below I've added suggestions for a quick fix for both these issues to get the test to pass... I'll let you decide if we keep these hacks and merge the PR or leave it open to find better solutions...

@alexhroom alexhroom requested a review from mducle December 23, 2025 11:37
Copy link
Member

@mducle mducle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, tests passed!

@mducle mducle merged commit 6a8723b into main Dec 23, 2025
13 checks passed
@mducle mducle deleted the 76-spin-spin branch December 23, 2025 11:45
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.

Add spin-spin correlation function to spinwave calculation

3 participants