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

Copy eigenvalues from D to omega when hermit=false and no mex #182

Merged

Conversation

RichardWaiteSTFC
Copy link
Collaborator

@RichardWaiteSTFC RichardWaiteSTFC commented May 2, 2024

Testing Instructions

(1) Run this script (taken from Tutorial 19)

swpref.setpref('usemex', false);

FeCuChain = spinw;
FeCuChain.genlattice('lat_const',[3 8 4],'spgr','P 1')
FeCuChain.addatom('label','MCu2','r',[0 0 0])
FeCuChain.addatom('label','MFe2','r',[0 1/2 0])

FeCuChain.gencoupling
FeCuChain.addmatrix('label','J_{Cu-Cu}','value',1,'color','r')
FeCuChain.addmatrix('label','J_{Fe-Fe}','value',1,'color','b')
FeCuChain.addmatrix('label','J_{Cu-Fe}','value',-0.1,'color','orange')

FeCuChain.addcoupling('mat','J_{Cu-Cu}','bond',1)
FeCuChain.addcoupling('mat','J_{Fe-Fe}','bond',2)
FeCuChain.addcoupling('mat','J_{Cu-Fe}','bond',[4 5])
FeCuChain.genmagstr('mode','helical','S',[0 0;1 1;0 0],'k',[1/2 0 0])

spec = FeCuChain.spinwave({[0 0 0] [1 0 0] 21},'formfact',true,'hermit',false, ...
                          'fastmode', true);

f = figure('color','white');
ax = axes(f); box on; hold on;
plot(ax, spec.hkl(1,:), spec.omega,'marker', 'o')
xlabel(ax, "H (r.l.u.)")
ylabel(ax, "Energy (meV)")

It should produce this plot
image

Previously all eigenvalues were 0

Fixes #181

Copy link

github-actions bot commented May 2, 2024

Test Results

    4 files  ± 0    104 suites   - 2   8m 0s ⏱️ - 7m 23s
  684 tests  - 19    666 ✅  - 19  18 💤 ±0  0 ❌ ±0 
1 908 runs   - 38  1 872 ✅  - 38  36 💤 ±0  0 ❌ ±0 

Results for commit 2b98670. ± Comparison against base commit 74422b7.

This pull request removes 19 tests.
sw_tests.system_tests.systemtest_spinwave_symbolic_nips ‑ test_symbolic_general_hkl
sw_tests.system_tests.systemtest_spinwave_symbolic_nips ‑ test_symbolic_hamiltonian
sw_tests.system_tests.systemtest_spinwave_symbolic_nips ‑ test_symbolic_hamiltonian_squared
sw_tests.system_tests.systemtest_spinwave_symbolic_nips ‑ test_symbolic_hamiltonian_white
sw_tests.system_tests.systemtest_spinwave_symbolic_nips ‑ test_symbolic_spectra(test_spectra_function_name=sw_egrid)
sw_tests.system_tests.systemtest_spinwave_symbolic_nips ‑ test_symbolic_spectra(test_spectra_function_name=sw_instrument)
sw_tests.system_tests.systemtest_spinwave_symbolic_nips ‑ test_symbolic_spectra(test_spectra_function_name=sw_neutron)
sw_tests.system_tests.systemtest_spinwave_symbolic_nips ‑ test_symbolic_spectra(test_spectra_function_name=sw_omegasum)
sw_tests.system_tests.systemtest_spinwave_symbolic_nips ‑ test_symbolic_spectra(test_spectra_function_name=sw_plotspec)
sw_tests.system_tests.systemtest_spinwave_symbolic_nips ‑ test_symbolic_spectra(test_spectra_function_name=sw_tofres)
…

♻️ This comment has been updated with latest results.

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.

Um, could you also remove lines 1163-1165 please? (I think I made a typo there and this supersedes it). Also maybe you can cherry pick the CI changes (commit 472e474c from the other PR and then merge this PR?

Was getting error from hermit=true as D not assigned
@RichardWaiteSTFC RichardWaiteSTFC force-pushed the 181_fix_fastmode_hermit_false_bug_not_copying_eigvals branch from b05c12c to 5b5f4c2 Compare May 3, 2024 08:43
@RichardWaiteSTFC
Copy link
Collaborator Author

Um, could you also remove lines 1163-1165 please? (I think I made a typo there and this supersedes it). Also maybe you can cherry pick the CI changes (commit 472e474c from the other PR and then merge this PR?

The previous fix was causing tests to fail for hermit=true e.g.

================================================================================
  Error occurred in sw_tests.unit_tests.unittest_spinw_spinwave/test_fastmode(mex=char_old) and it did not run to completion.
      ---------
      Error ID:
      ---------
      'MATLAB:UndefinedFunction'
      --------------
      Error Details:
      --------------
      Unrecognized function or variable 'D'.
      
      Error in spinw/spinwave (line 1044)
              omega = D(1:nMagExt,:);
      
      Error in sw_tests.unit_tests.unittest_spinw_spinwave/test_fastmode (line 657)
                  spec1 = swobj.spinwave(hkl, 'fastmode', true);
  ================================================================================

So after this commit 5b5f4c2 I think I need to keep lines 1163-1165 (also from code looks like this will also be needed is using memory management loop).

RichardWaiteSTFC and others added 2 commits May 3, 2024 10:03
Note only the updated test_fastmode would fail on main branch
Update base github-actions to node-20 (v4)
   gha/[up|down]load-artifact no longer accept
   same named artifacts for a matrix run
Update Matlab CI to v2
   lowest supported version is R2021a
Update sw_mex to support Apple-Silicon (arm64)
Fix bug in INSTALL_DEPS in yaml
Inc pcsmo test tolerance to pass on MacOS-14
@RichardWaiteSTFC RichardWaiteSTFC merged commit c046e97 into master May 3, 2024
15 checks passed
@mducle mducle deleted the 181_fix_fastmode_hermit_false_bug_not_copying_eigvals branch May 30, 2024 02:01
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.

Bug in fastmode spinwave calculation when mex disabled and hermit=false
2 participants