SpinW / spinw

SpinW Matlab library for spin wave calculation
http://www.spinw.org
GNU General Public License v3.0
37 stars 15 forks source link

Fix out-of-memory error in mmat #133

Closed mducle closed 1 year ago

mducle commented 1 year ago

In order to speed up the calculation of the matrix-matrix multiplication of a stack of matrices with just pure Matlab code, the computation the mmat function expands the matrices so that it can just use a single call to bsxfun to compute the element-wise multiplication in parallel followed by a single call to sum.

However, this requires very large temporary arrays such that the system very rapidly runs out of memory - for example, multiplying an N×M by a stack M×N×L of L matrices requires two temporary arrays of size N×M×N×L matrices.

This PR changes the code so that if the memory requirements are too much, it instead computes the stacked matrix multiplication using loops which would be slow but at least the calculation will actually run.

github-actions[bot] commented 1 year ago

Test Results

       4 files  ±    0       98 suites  +4   19m 26s :stopwatch: + 11m 19s    583 tests +  77     583 :heavy_check_mark: +  77  0 :zzz: ±0  0 :x: ±0  1 622 runs  +108  1 622 :heavy_check_mark: +108  0 :zzz: ±0  0 :x: ±0 

Results for commit 688ecbc5. ± Comparison against base commit 9c713139.

This pull request removes 28 and adds 105 tests. Note that renamed tests count towards both. ``` sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_cmplxBase_equivalent_with_tri sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_cmplxBase_fails_with_chain sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_formfact sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_formfactfun sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_gtensor sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_gtensor_incomm sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_hermit sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_incommensurate sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_notwin sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_sw_incom_in_supercell_warns … ``` ``` sw_tests.unit_tests.unittest_mmat ‑ test_mmat_branch sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_cmplxBase_equivalent_with_tri(mex=0) sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_cmplxBase_equivalent_with_tri(mex=1) sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_cmplxBase_equivalent_with_tri(mex=value1) sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_cmplxBase_equivalent_with_tri(mex=value2) sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_cmplxBase_fails_with_chain(mex=0) sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_cmplxBase_fails_with_chain(mex=1) sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_cmplxBase_fails_with_chain(mex=value1) sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_cmplxBase_fails_with_chain(mex=value2) sw_tests.unit_tests.unittest_spinw_spinwave ‑ test_formfact(mex=0) … ```

:recycle: This comment has been updated with latest results.

codecov-commenter commented 1 year ago

Codecov Report

Merging #133 (bc043d9) into master (9c71313) will decrease coverage by 0.03%. The diff coverage is 22.22%.

@@            Coverage Diff             @@
##           master     #133      +/-   ##
==========================================
- Coverage   38.73%   38.70%   -0.03%     
==========================================
  Files         239      239              
  Lines       15795    15822      +27     
==========================================
+ Hits         6118     6124       +6     
- Misses       9677     9698      +21     
Impacted Files Coverage Δ
external/mmat.m 47.91% <22.22%> (-33.04%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

RichardWaiteSTFC commented 1 year ago

... is it worth adding a quick unit test looping over the size of arrays doing e.g. A * A^-1 = I (possibly repeated over a third dimension for one of the arrays) ?

Second thoughts it's hard to guarantee the computer won't have enough ram unless you make this very big and then it might take quite a while...

mducle commented 1 year ago

... is it worth adding a quick unit test looping over the size of arrays doing e.g. A * A^-1 = I (possibly repeated over a third dimension for one of the arrays) ?

Second thoughts it's hard to guarantee the computer won't have enough ram unless you make this very big and then it might take quite a while...

No it's fine... I just need to mock sw_freemem to return a low value so the check automatically passes... I just spotted a bug in the mock_function utility so have to fix that first...

wardsimon commented 1 year ago

It all looks good to me, however can you check with the memory profiler that your estimate for memory usage is correct. The implementation of repmat & permute does use the memory estimated, however I the next line using sumsym & bsxfunsym does not do calculations in place (matrix C and B are in memory), leading to a doubling of memory usage.

For the test case: Estimated memory usage = 591kB repmat & permute = 612.89 kB used (0 kB freed) sumsym & bsxfunsym = 611.92 kB used (64.44 kb freed)

So maybe multiply the memory estimate by 2.

mducle commented 1 year ago

@wardsimon Thanks for this. I had 16 bytes per element before - I think I got it from another bit of the code but couldn't remember why. I've updated the code to use this now, and also added your explanation.