From 5c3c9847548197871f9f1f3e0489b157aac7a372 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Mon, 11 Jul 2022 11:50:40 +0100 Subject: [PATCH 01/41] Add initial genmagstr test file --- .../+unit_tests/unittest_spinw_genmagstr.m | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 +sw_tests/+unit_tests/unittest_spinw_genmagstr.m diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m new file mode 100644 index 00000000..a14ca496 --- /dev/null +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -0,0 +1,21 @@ +classdef unittest_spinw_genmagstr < sw_tests.unit_tests.unittest_super + + properties + swobj = []; + end + + methods (TestMethodSetup) + function setup_spinw_model(testCase) + testCase.swobj = spinw(); % default init + end + end + + methods (Test) + function test_no_magnetic_atoms_raises_error(testCase) + testCase.verifyError(... + @() testCase.swobj.genmagstr(), ... + 'spinw:genmagstr:NoMagAtom') + end + end + +end From bd3b149773c5bf0fcf081fbcd0abb4159817c6d3 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Mon, 11 Jul 2022 14:35:19 +0100 Subject: [PATCH 02/41] Add initial simple comm/incomm tests --- .../+unit_tests/unittest_spinw_genmagstr.m | 50 +++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index a14ca496..e432b8d5 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -2,20 +2,64 @@ properties swobj = []; + swobj_tri = []; + default_magstr = struct('S', [0; 1; 0], ... + 'k', [0 0 0], ... + 'n', [0 0 1], ... + 'N_ext', [1 1 1], ... + 'exact', true); end methods (TestMethodSetup) - function setup_spinw_model(testCase) - testCase.swobj = spinw(); % default init + function setup_chain_model(testCase) + % Create a simple FM 1D chain model + testCase.swobj = spinw(); + testCase.swobj.genlattice('lat_const', [3 8 8], 'angled', [90 90 90]); + testCase.swobj.addatom('r', [0 0 0],'S', 1, 'label', 'MNi2'); + testCase.swobj.gencoupling('maxDistance', 7); + testCase.swobj.addmatrix('value', -eye(3), 'label', 'Ja'); + testCase.swobj.addcoupling('mat', 'Ja', 'bond', 1); + end + function setup_tri_model(testCase) + % Create a simple triangular lattice model + testCase.swobj_tri = spinw; + testCase.swobj_tri.genlattice('lat_const', [4 4 6], 'angled', [90 90 120]); + testCase.swobj_tri.addatom('r', [0 0 0], 'S', 3/2, 'label', 'MCr3'); + J1 = 1; + testCase.swobj_tri.addmatrix('label','J1','value',J1); + testCase.swobj_tri.gencoupling; + testCase.swobj_tri.addcoupling('mat','J1','bond',1); end end methods (Test) function test_no_magnetic_atoms_raises_error(testCase) + swobj = spinw; testCase.verifyError(... - @() testCase.swobj.genmagstr(), ... + @() swobj.genmagstr(), ... 'spinw:genmagstr:NoMagAtom') end + function test_chain(testCase) + swobj = copy(testCase.swobj); + swobj.genmagstr('mode','direct', 'k',[0 0 0],'n',[1 0 0],'S',[0; 1; 0]); + testCase.verify_obj(testCase.default_magstr, swobj.magstr); + + end + function test_chain_nok(testCase) + swobj = copy(testCase.swobj); + swobj.genmagstr('mode','direct', 'n',[1 0 0],'S',[0; 1; 0]); + testCase.verify_obj(testCase.default_magstr, swobj.magstr); + end + function test_tri(testCase) + swobj_tri = copy(testCase.swobj_tri); + swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... + 'n', [0 0 1], 'k', [1/3 1/3 0]); + expected_magstr = testCase.default_magstr; + expected_magstr.S = [1.5; 0; 0]; + expected_magstr.k = [1/3 1/3 0]; + testCase.verify_obj(expected_magstr, swobj_tri.magstr); + + end end end From d69bc37244867bdfdaca48757c24da4997fe47e2 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Mon, 11 Jul 2022 15:15:23 +0100 Subject: [PATCH 03/41] Add zero nExt test --- +sw_tests/+unit_tests/unittest_spinw_genmagstr.m | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index e432b8d5..99d4d33e 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -39,6 +39,11 @@ function test_no_magnetic_atoms_raises_error(testCase) @() swobj.genmagstr(), ... 'spinw:genmagstr:NoMagAtom') end + function test_zero_nExt_raises_error(testCase) + testCase.verifyError(... + @() testCase.swobj.genmagstr('nExt', [0 1 1]), ... + 'spinw:genmagstr:WrongInput') + end function test_chain(testCase) swobj = copy(testCase.swobj); swobj.genmagstr('mode','direct', 'k',[0 0 0],'n',[1 0 0],'S',[0; 1; 0]); From c526f521f5606b1080ffe39c3d0170e356c136fb Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Tue, 12 Jul 2022 15:28:21 +0100 Subject: [PATCH 04/41] Add afm chain test --- .../+unit_tests/unittest_spinw_genmagstr.m | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 99d4d33e..759bb019 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -16,19 +16,12 @@ function setup_chain_model(testCase) testCase.swobj = spinw(); testCase.swobj.genlattice('lat_const', [3 8 8], 'angled', [90 90 90]); testCase.swobj.addatom('r', [0 0 0],'S', 1, 'label', 'MNi2'); - testCase.swobj.gencoupling('maxDistance', 7); - testCase.swobj.addmatrix('value', -eye(3), 'label', 'Ja'); - testCase.swobj.addcoupling('mat', 'Ja', 'bond', 1); end function setup_tri_model(testCase) % Create a simple triangular lattice model testCase.swobj_tri = spinw; testCase.swobj_tri.genlattice('lat_const', [4 4 6], 'angled', [90 90 120]); testCase.swobj_tri.addatom('r', [0 0 0], 'S', 3/2, 'label', 'MCr3'); - J1 = 1; - testCase.swobj_tri.addmatrix('label','J1','value',J1); - testCase.swobj_tri.gencoupling; - testCase.swobj_tri.addcoupling('mat','J1','bond',1); end end @@ -65,6 +58,17 @@ function test_tri(testCase) testCase.verify_obj(expected_magstr, swobj_tri.magstr); end + function test_afm_chain(testCase) + afm_chain = copy(testCase.swobj); + afm_chain.genmagstr('mode', 'direct', 'k',[1/2 0 0], ... + 'n',[1 0 0],'S',[0 0; 1 -1;0 0], ... + 'nExt',[2 1 1]); + expected_magstr = testCase.default_magstr; + expected_magstr.S = [0 0; 1 -1; 0 0]; + expected_magstr.N_ext = [2 1 1]; + testCase.verify_obj(expected_magstr, afm_chain.magstr); + + end end -end +end \ No newline at end of file From edc54ded9f91491721fc961bf28613a5e42c9c39 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Thu, 14 Jul 2022 15:09:51 +0100 Subject: [PATCH 05/41] Add random structure tests --- .../+unit_tests/unittest_spinw_genmagstr.m | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 759bb019..944e6522 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -37,18 +37,18 @@ function test_zero_nExt_raises_error(testCase) @() testCase.swobj.genmagstr('nExt', [0 1 1]), ... 'spinw:genmagstr:WrongInput') end - function test_chain(testCase) + function test_direct_fm_chain(testCase) swobj = copy(testCase.swobj); swobj.genmagstr('mode','direct', 'k',[0 0 0],'n',[1 0 0],'S',[0; 1; 0]); testCase.verify_obj(testCase.default_magstr, swobj.magstr); end - function test_chain_nok(testCase) + function test_direct_fm_chain_nok(testCase) swobj = copy(testCase.swobj); swobj.genmagstr('mode','direct', 'n',[1 0 0],'S',[0; 1; 0]); testCase.verify_obj(testCase.default_magstr, swobj.magstr); end - function test_tri(testCase) + function test_helical_tri(testCase) swobj_tri = copy(testCase.swobj_tri); swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... 'n', [0 0 1], 'k', [1/3 1/3 0]); @@ -58,7 +58,7 @@ function test_tri(testCase) testCase.verify_obj(expected_magstr, swobj_tri.magstr); end - function test_afm_chain(testCase) + function test_direct_afm_chain(testCase) afm_chain = copy(testCase.swobj); afm_chain.genmagstr('mode', 'direct', 'k',[1/2 0 0], ... 'n',[1 0 0],'S',[0 0; 1 -1;0 0], ... @@ -69,6 +69,40 @@ function test_afm_chain(testCase) testCase.verify_obj(expected_magstr, afm_chain.magstr); end + function test_random_structure(testCase) + swobj = copy(testCase.swobj); + swobj.genmagstr('mode','random'); + magstr1 = swobj.magstr; + swobj.genmagstr('mode','random'); + magstr2 = swobj.magstr; + % Check structure is random each time - S is different + testCase.verifyNotEqual(magstr1.S, magstr2.S); + % Check S has correct magnitude + testCase.verify_val(norm(magstr1.S, 2), 1); + % Check other fields + expected_magstr = testCase.default_magstr; + testCase.verify_obj(rmfield(expected_magstr, 'S'), ... + rmfield(magstr1, 'S')); + end + function test_random_structure_multiatom_and_nExt(testCase) + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0],'S', 2); + nExt = [2 2 2]; + swobj.genmagstr('mode','random', 'nExt', nExt); + magstr1 = swobj.magstr; + swobj.genmagstr('mode','random', 'nExt', nExt); + magstr2 = swobj.magstr; + % Check structure is random each time - S is different + testCase.verifyNotEqual(magstr1.S, magstr2.S); + % Check S has correct magnitudes + testCase.verify_val(vecnorm(magstr1.S(:, 1:2:end), 2), ones(1, 8)); + testCase.verify_val(vecnorm(magstr1.S(:, 2:2:end), 2), 2*ones(1, 8)); + % Check other fields + expected_magstr = testCase.default_magstr; + expected_magstr.N_ext = nExt; + testCase.verify_obj(rmfield(expected_magstr, 'S'), ... + rmfield(magstr1, 'S')); + end end end \ No newline at end of file From 4ef8e4c13f63c253194c2474e13b61e5a4dd575e Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Fri, 15 Jul 2022 09:40:49 +0100 Subject: [PATCH 06/41] Check mag_str not magstr --- .../+unit_tests/unittest_spinw_genmagstr.m | 82 ++++++++++--------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 944e6522..8f99ba8a 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -3,11 +3,9 @@ properties swobj = []; swobj_tri = []; - default_magstr = struct('S', [0; 1; 0], ... - 'k', [0 0 0], ... - 'n', [0 0 1], ... - 'N_ext', [1 1 1], ... - 'exact', true); + default_mag_str = struct('nExt', ones(1, 3, 'int32'), ... + 'k', [0; 0; 0], ... + 'F', [0; 1; 0]); end methods (TestMethodSetup) @@ -39,23 +37,24 @@ function test_zero_nExt_raises_error(testCase) end function test_direct_fm_chain(testCase) swobj = copy(testCase.swobj); - swobj.genmagstr('mode','direct', 'k',[0 0 0],'n',[1 0 0],'S',[0; 1; 0]); - testCase.verify_obj(testCase.default_magstr, swobj.magstr); + swobj.genmagstr('mode', 'direct', 'k', [0 0 0], ... + 'n', [1 0 0], 'S', [0; 1; 0]); + testCase.verify_obj(testCase.default_mag_str, swobj.mag_str); end function test_direct_fm_chain_nok(testCase) swobj = copy(testCase.swobj); - swobj.genmagstr('mode','direct', 'n',[1 0 0],'S',[0; 1; 0]); - testCase.verify_obj(testCase.default_magstr, swobj.magstr); + swobj.genmagstr('mode', 'direct', 'n', [1 0 0], 'S', [0; 1; 0]); + testCase.verify_obj(testCase.default_mag_str, swobj.mag_str); end function test_helical_tri(testCase) swobj_tri = copy(testCase.swobj_tri); swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... - 'n', [0 0 1], 'k', [1/3 1/3 0]); - expected_magstr = testCase.default_magstr; - expected_magstr.S = [1.5; 0; 0]; - expected_magstr.k = [1/3 1/3 0]; - testCase.verify_obj(expected_magstr, swobj_tri.magstr); + 'n', [0 0 1], 'k', [1/3 1/3 0]); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.k = [1/3; 1/3; 0]; + expected_mag_str.F = [1.5; 1.5j; 0]; + testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); end function test_direct_afm_chain(testCase) @@ -63,45 +62,48 @@ function test_direct_afm_chain(testCase) afm_chain.genmagstr('mode', 'direct', 'k',[1/2 0 0], ... 'n',[1 0 0],'S',[0 0; 1 -1;0 0], ... 'nExt',[2 1 1]); - expected_magstr = testCase.default_magstr; - expected_magstr.S = [0 0; 1 -1; 0 0]; - expected_magstr.N_ext = [2 1 1]; - testCase.verify_obj(expected_magstr, afm_chain.magstr); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.nExt(1) = int32(2); + expected_mag_str.k = [0.5; 0; 0]; + expected_mag_str.F = [0 0; 1 -1; 0 0]; + testCase.verify_obj(expected_mag_str, afm_chain.mag_str); end function test_random_structure(testCase) swobj = copy(testCase.swobj); swobj.genmagstr('mode','random'); - magstr1 = swobj.magstr; + mag_str1 = swobj.mag_str; swobj.genmagstr('mode','random'); - magstr2 = swobj.magstr; - % Check structure is random each time - S is different - testCase.verifyNotEqual(magstr1.S, magstr2.S); + mag_str2 = swobj.mag_str; + % Check structure is random each time - F is different + testCase.verifyNotEqual(mag_str1.F, mag_str2.F); % Check S has correct magnitude - testCase.verify_val(norm(magstr1.S, 2), 1); + testCase.verify_val(norm(swobj.magstr.S, 2), 1); % Check other fields - expected_magstr = testCase.default_magstr; - testCase.verify_obj(rmfield(expected_magstr, 'S'), ... - rmfield(magstr1, 'S')); + expected_mag_str = testCase.default_mag_str; + testCase.verify_obj(rmfield(expected_mag_str, 'F'), ... + rmfield(mag_str1, 'F')); end function test_random_structure_multiatom_and_nExt(testCase) swobj = copy(testCase.swobj); - swobj.addatom('r', [0.5 0.5 0],'S', 2); - nExt = [2 2 2]; - swobj.genmagstr('mode','random', 'nExt', nExt); - magstr1 = swobj.magstr; - swobj.genmagstr('mode','random', 'nExt', nExt); - magstr2 = swobj.magstr; - % Check structure is random each time - S is different - testCase.verifyNotEqual(magstr1.S, magstr2.S); + swobj.addatom('r', [0.5 0.5 0], 'S', 2); + nExt = 2*ones(1, 3, 'int32'); + swobj.genmagstr('mode', 'random', 'nExt', nExt); + mag_str1 = swobj.mag_str; + swobj.genmagstr('mode', 'random', 'nExt', nExt); + mag_str2 = swobj.mag_str; + % Check structure is random each time - F is different + testCase.verifyNotEqual(mag_str1.F, mag_str2.F); % Check S has correct magnitudes - testCase.verify_val(vecnorm(magstr1.S(:, 1:2:end), 2), ones(1, 8)); - testCase.verify_val(vecnorm(magstr1.S(:, 2:2:end), 2), 2*ones(1, 8)); + testCase.verify_val(vecnorm(swobj.magstr.S(:, 1:2:end), 2), ... + ones(1, 8)); + testCase.verify_val(vecnorm(swobj.magstr.S(:, 2:2:end), 2), ... + 2*ones(1, 8)); % Check other fields - expected_magstr = testCase.default_magstr; - expected_magstr.N_ext = nExt; - testCase.verify_obj(rmfield(expected_magstr, 'S'), ... - rmfield(magstr1, 'S')); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.nExt = nExt; + testCase.verify_obj(rmfield(expected_mag_str, 'F'), ... + rmfield(mag_str1, 'F')); end end From c13a776eccb193b80ddf3f4c329e0ed745aa890c Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Fri, 15 Jul 2022 14:27:54 +0100 Subject: [PATCH 07/41] Add random test with k --- .../+unit_tests/unittest_spinw_genmagstr.m | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 8f99ba8a..7249c6e8 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -77,13 +77,29 @@ function test_random_structure(testCase) mag_str2 = swobj.mag_str; % Check structure is random each time - F is different testCase.verifyNotEqual(mag_str1.F, mag_str2.F); - % Check S has correct magnitude + testCase.verifyEqual(size(mag_str1.F), size(mag_str2.F)); + % Check S has correct size and magnitude testCase.verify_val(norm(swobj.magstr.S, 2), 1); % Check other fields expected_mag_str = testCase.default_mag_str; testCase.verify_obj(rmfield(expected_mag_str, 'F'), ... rmfield(mag_str1, 'F')); end + function test_random_structure_k(testCase) + swobj = copy(testCase.swobj); + k = [0; 0; 1/4]; + swobj.genmagstr('mode','random', 'k', k'); + mag_str1 = swobj.mag_str; + % Check F size + testCase.verifySize(mag_str1.F, [3 1]); + % Check S has correct size and magnitude + testCase.verify_val(vecnorm(swobj.magstr.S, 2), 1); + % Check other fields + expected_mag_str = testCase.default_mag_str; + expected_mag_str.k = k; + testCase.verify_obj(rmfield(expected_mag_str, 'F'), ... + rmfield(mag_str1, 'F')); + end function test_random_structure_multiatom_and_nExt(testCase) swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0], 'S', 2); @@ -94,6 +110,7 @@ function test_random_structure_multiatom_and_nExt(testCase) mag_str2 = swobj.mag_str; % Check structure is random each time - F is different testCase.verifyNotEqual(mag_str1.F, mag_str2.F); + testCase.verifySize(mag_str1.F, [3 16]); % Check S has correct magnitudes testCase.verify_val(vecnorm(swobj.magstr.S(:, 1:2:end), 2), ... ones(1, 8)); From 06839c931c7dc8c0295ced8dc8db79dedd5498c3 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Mon, 25 Jul 2022 12:39:23 +0100 Subject: [PATCH 08/41] Add test with existing k --- +sw_tests/+unit_tests/unittest_spinw_genmagstr.m | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 7249c6e8..c7bf7f63 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -48,6 +48,18 @@ function test_direct_fm_chain_nok(testCase) testCase.verify_obj(testCase.default_mag_str, swobj.mag_str); end function test_helical_tri(testCase) + swobj_tri = copy(testCase.swobj_tri); + swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... + 'n', [0 0 1], 'k', [1/3 1/3 0]); + swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... + 'n', [0 0 1], 'k', [1/3 1/3 0]); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.k = [1/3; 1/3; 0]; + expected_mag_str.F = [1.5; 1.5j; 0]; + testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); + + end + function test_helical_tri_existing_k(testCase) swobj_tri = copy(testCase.swobj_tri); swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... 'n', [0 0 1], 'k', [1/3 1/3 0]); From 953ce0e57be46d6893d102911e24710dce1fd345 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Mon, 25 Jul 2022 16:07:18 +0100 Subject: [PATCH 09/41] Add multiatom direct test --- .../+unit_tests/unittest_spinw_genmagstr.m | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index c7bf7f63..dfd3e631 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -35,6 +35,11 @@ function test_zero_nExt_raises_error(testCase) @() testCase.swobj.genmagstr('nExt', [0 1 1]), ... 'spinw:genmagstr:WrongInput') end + function test_direct_wrong_spin_size_raises_error(testCase) + testCase.verifyError(... + @() testCase.swobj.genmagstr('mode', 'direct', 'S', [0; 1; 0], 'nExt', [2 1 1]), ... + 'spinw:genmagstr:WrongSpinSize') + end function test_direct_fm_chain(testCase) swobj = copy(testCase.swobj); swobj.genmagstr('mode', 'direct', 'k', [0 0 0], ... @@ -47,6 +52,37 @@ function test_direct_fm_chain_nok(testCase) swobj.genmagstr('mode', 'direct', 'n', [1 0 0], 'S', [0; 1; 0]); testCase.verify_obj(testCase.default_mag_str, swobj.mag_str); end + function test_direct_multiatom_nExt(testCase) + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0.5], 'S', 2); + S = [1 0 1 -1; ... + 1 1 0 0; ... + 0 -1 0 0]; + nExt = [int32(2) int32(1) int32(1)]; + k = [0 1/3 0]; + swobj.genmagstr('mode', 'direct', 'S', S, 'nExt', nExt, 'k', k); + expected_mag_str = struct('nExt', nExt, ... + 'k', k', ... + 'F', [sqrt(2)/2 0 1 -2; ... + sqrt(2)/2 sqrt(2) 0 0; ... + 0 -sqrt(2) 0 0]); + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_direct_multiatom_multik(testCase) + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0.5], 'S', 2); + S_k = [1 0; 1 1; 0 -1]; + S = cat(3, S_k, S_k); + k = [0 1/3 0; 1/2 0 0]; + swobj.genmagstr('mode', 'direct', 'S', S, 'k', k); + F_k = [sqrt(2)/2 0; ... + sqrt(2)/2 sqrt(2); ... + 0 -sqrt(2)]; + expected_mag_str = testCase.default_mag_str; + expected_mag_str.k = k'; + expected_mag_str.F = cat(3, F_k, F_k); + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end function test_helical_tri(testCase) swobj_tri = copy(testCase.swobj_tri); swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... From cfdcdfe8782ca759f754a17c6e6aa8be892eb5f2 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Tue, 26 Jul 2022 16:55:25 +0100 Subject: [PATCH 10/41] Add helical tests, fix multi-k bug In the if ~cmplxS... conditional any(k) returns a vector for multi-k structures, causing a MATLAB:nonLogicalConditional error, fix this by flattening k first --- .../+unit_tests/unittest_spinw_genmagstr.m | 85 +++++++++++++------ CHANGELOG.rst | 2 + swfiles/@spinw/genmagstr.m | 2 +- 3 files changed, 61 insertions(+), 28 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index dfd3e631..7472fef3 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -13,13 +13,13 @@ function setup_chain_model(testCase) % Create a simple FM 1D chain model testCase.swobj = spinw(); testCase.swobj.genlattice('lat_const', [3 8 8], 'angled', [90 90 90]); - testCase.swobj.addatom('r', [0 0 0],'S', 1, 'label', 'MNi2'); + testCase.swobj.addatom('r', [0 0 0],'S', 1); end function setup_tri_model(testCase) % Create a simple triangular lattice model testCase.swobj_tri = spinw; testCase.swobj_tri.genlattice('lat_const', [4 4 6], 'angled', [90 90 120]); - testCase.swobj_tri.addatom('r', [0 0 0], 'S', 3/2, 'label', 'MCr3'); + testCase.swobj_tri.addatom('r', [0 0 0], 'S', 3/2); end end @@ -37,19 +37,28 @@ function test_zero_nExt_raises_error(testCase) end function test_direct_wrong_spin_size_raises_error(testCase) testCase.verifyError(... - @() testCase.swobj.genmagstr('mode', 'direct', 'S', [0; 1; 0], 'nExt', [2 1 1]), ... + @() testCase.swobj.genmagstr( ... + 'mode', 'direct', 'S', [0; 1; 0], 'nExt', [2 1 1]), ... 'spinw:genmagstr:WrongSpinSize') end + function test_helical_wrong_spin_size_warns(testCase) + testCase.verifyWarning(... + @() testCase.swobj.genmagstr('mode', 'helical', ... + 'S', [1 0; 0 1; 0 0], ... + 'k', [1/3 0 0], ... + 'nExt', [2 1 1]), ... + 'spinw:genmagstr:UCExtNonSuff') + end function test_direct_fm_chain(testCase) swobj = copy(testCase.swobj); swobj.genmagstr('mode', 'direct', 'k', [0 0 0], ... - 'n', [1 0 0], 'S', [0; 1; 0]); + 'S', [0; 1; 0]); testCase.verify_obj(testCase.default_mag_str, swobj.mag_str); end function test_direct_fm_chain_nok(testCase) swobj = copy(testCase.swobj); - swobj.genmagstr('mode', 'direct', 'n', [1 0 0], 'S', [0; 1; 0]); + swobj.genmagstr('mode', 'direct', 'S', [0; 1; 0]); testCase.verify_obj(testCase.default_mag_str, swobj.mag_str); end function test_direct_multiatom_nExt(testCase) @@ -86,36 +95,58 @@ function test_direct_multiatom_multik(testCase) function test_helical_tri(testCase) swobj_tri = copy(testCase.swobj_tri); swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... - 'n', [0 0 1], 'k', [1/3 1/3 0]); - swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... - 'n', [0 0 1], 'k', [1/3 1/3 0]); + 'k', [1/3 1/3 0]); expected_mag_str = testCase.default_mag_str; expected_mag_str.k = [1/3; 1/3; 0]; expected_mag_str.F = [1.5; 1.5j; 0]; testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); - end - function test_helical_tri_existing_k(testCase) - swobj_tri = copy(testCase.swobj_tri); - swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... - 'n', [0 0 1], 'k', [1/3 1/3 0]); - expected_mag_str = testCase.default_mag_str; - expected_mag_str.k = [1/3; 1/3; 0]; - expected_mag_str.F = [1.5; 1.5j; 0]; - testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); - + function test_helical_multiatom_nExt_1spin(testCase) + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0.0], 'S', 2); + k = [0 0 1/2]; + nExt = [int32(1) int32(1) int32(2)]; + swobj.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... + 'nExt', nExt, 'k', k); + expected_mag_str = struct(... + 'k', k', ... + 'nExt', nExt, ... + 'F', [1 2 -1 -2; 1i 2i -1i -2i; 0 0 0 0]); + testCase.verify_obj(expected_mag_str, swobj.mag_str); end - function test_direct_afm_chain(testCase) - afm_chain = copy(testCase.swobj); - afm_chain.genmagstr('mode', 'direct', 'k',[1/2 0 0], ... - 'n',[1 0 0],'S',[0 0; 1 -1;0 0], ... - 'nExt',[2 1 1]); + function test_helical_multiatom_nExt_1spin_r0(testCase) + swobj = spinw(); + swobj.genlattice('lat_const', [3 8 8], 'angled', [90 90 90]); + % Need to have nonzero r for first atom for r0 to have an effect + swobj.addatom('r', [0.5 0.5 0.5],'S', 1); + swobj.addatom('r', [0 0 0], 'S', 2); + k = [0 0 1/2]; + nExt = [int32(1) int32(1) int32(2)]; + swobj.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... + 'nExt', nExt, 'k', k, 'r0', false); + expected_mag_str = struct(... + 'k', k', ... + 'nExt', nExt, ... + 'F', [1 2i -1 -2i; 1i -2 -1i 2; 0 0 0 0]); + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_helical_multiatom_multik(testCase) + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0.5], 'S', 2); + S_k = [1; 0; 0]; + S = cat(3, S_k, S_k); + k = [0 1/3 0; 1/2 0 0]; + swobj.genmagstr('mode', 'helical', 'S', S, 'k', k); + F_k = [sqrt(2)/2 0; ... + sqrt(2)/2 sqrt(2); ... + 0 -sqrt(2)]; expected_mag_str = testCase.default_mag_str; - expected_mag_str.nExt(1) = int32(2); - expected_mag_str.k = [0.5; 0; 0]; - expected_mag_str.F = [0 0; 1 -1; 0 0]; - testCase.verify_obj(expected_mag_str, afm_chain.mag_str); + expected_mag_str.k = k'; + expected_mag_str.F = cat(3, ... + [1 1-sqrt(3)*i; 1i sqrt(3)+1i; 0 0], ... + [1 -2i; 1i 2; 0 0]); + testCase.verify_obj(expected_mag_str, swobj.mag_str); end function test_random_structure(testCase) swobj = copy(testCase.swobj); diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b1d54515..83245643 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -33,3 +33,5 @@ Bug Fixes length < ``maxSym`` - A warning will now be emitted if ``saveSabp`` is requested in ``spinwave`` for a commensurate structure +- Fix ``MATLAB:nonLogicalConditional`` error raised when using multiple + k in ``genmagstr`` with ``helical`` mode diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index 4b847bf7..abd6a355 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -315,7 +315,7 @@ function genmagstr(obj, varargin) n = sym(n); end -if ~cmplxS && ~strcmpi(param.mode,'fourier') && ~strcmpi(param.mode,'direct') && any(k) +if ~cmplxS && ~strcmpi(param.mode,'fourier') && ~strcmpi(param.mode,'direct') && any(k(:)) param.S = param.S + 1i*cross(repmat(permute(n,[2 3 1]),[1 size(param.S,2) 1]),param.S); end From bf4105c5004cb62dbe7b02ff94e607b6688dd2ed Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Wed, 27 Jul 2022 09:58:29 +0100 Subject: [PATCH 11/41] Add to helical tests - Add test with multiple k and n - Add test with nSpin = nMagAtom - Add test with nSpin = nMagExt --- .../+unit_tests/unittest_spinw_genmagstr.m | 41 +++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 7472fef3..e942a694 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -102,6 +102,7 @@ function test_helical_tri(testCase) testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); end function test_helical_multiatom_nExt_1spin(testCase) + % Test where only 1 spin is provided swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0.0], 'S', 2); k = [0 0 1/2]; @@ -130,21 +131,53 @@ function test_helical_multiatom_nExt_1spin_r0(testCase) 'F', [1 2i -1 -2i; 1i -2 -1i 2; 0 0 0 0]); testCase.verify_obj(expected_mag_str, swobj.mag_str); end - function test_helical_multiatom_multik(testCase) + function test_helical_multiatom_nExt_nMagAtom_spins(testCase) + % Test where there are the same number of spins provided as in + % the unit cell + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0.0], 'S', 2); + k = [0 1/2 0]; + nExt = [int32(1) int32(2) int32(1)]; + swobj.genmagstr('mode', 'helical', 'S', [0 1; 1 0; 0 0], ... + 'nExt', nExt, 'k', k); + expected_mag_str = struct( ... + 'k', k', ... + 'nExt', nExt, ... + 'F', [-1i 2 1i -2; 1 2i -1 -2i; 0 0 0 0]); + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_helical_multiatom_nExt_nMagExt_spins(testCase) + % Test where there are the same number of spins provided as in + % the supercell + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0.0], 'S', 2); + k = [0 1/5 0]; + nExt = [int32(1) int32(2) int32(1)]; + swobj.genmagstr('mode', 'helical', ... + 'S', [0 1 0 -1; 1 0 0 0; 0 0 1 0], ... + 'nExt', nExt, 'k', k); + expected_mag_str = struct(... + 'k', k', ... + 'nExt', nExt, ... + 'F', [-1i 2 0 -2; 1 2i 0 -2i; 0 0 1 0]); + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_helical_multiatom_multik_multin(testCase) swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0.5], 'S', 2); S_k = [1; 0; 0]; S = cat(3, S_k, S_k); k = [0 1/3 0; 1/2 0 0]; - swobj.genmagstr('mode', 'helical', 'S', S, 'k', k); + n = [0 0 1; 0 1 0]; + swobj.genmagstr('mode', 'helical', 'S', S, 'k', k, 'n', n); F_k = [sqrt(2)/2 0; ... sqrt(2)/2 sqrt(2); ... 0 -sqrt(2)]; expected_mag_str = testCase.default_mag_str; expected_mag_str.k = k'; expected_mag_str.F = cat(3, ... - [1 1-sqrt(3)*i; 1i sqrt(3)+1i; 0 0], ... - [1 -2i; 1i 2; 0 0]); + [1 1-sqrt(3)*i; 1i sqrt(3)+1i; 0 0], ... + [1 -2i; 0 0; -1i -2]); testCase.verify_obj(expected_mag_str, swobj.mag_str); end From 39150eb48ddacba9909ad266b9054d20617b04da Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Wed, 27 Jul 2022 10:15:30 +0100 Subject: [PATCH 12/41] Add fourier tests --- .../+unit_tests/unittest_spinw_genmagstr.m | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index e942a694..31c24c22 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -98,7 +98,16 @@ function test_helical_tri(testCase) 'k', [1/3 1/3 0]); expected_mag_str = testCase.default_mag_str; expected_mag_str.k = [1/3; 1/3; 0]; - expected_mag_str.F = [1.5; 1.5j; 0]; + expected_mag_str.F = [1.5; 1.5i; 0]; + testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); + end + function test_fourier_tri(testCase) + swobj_tri = copy(testCase.swobj_tri); + swobj_tri.genmagstr('mode', 'fourier', 'S', [1; 0; 0], ... + 'k', [1/3 1/3 0]); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.k = [1/3; 1/3; 0]; + expected_mag_str.F = [1.5; 0; 0]; testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); end function test_helical_multiatom_nExt_1spin(testCase) @@ -131,6 +140,22 @@ function test_helical_multiatom_nExt_1spin_r0(testCase) 'F', [1 2i -1 -2i; 1i -2 -1i 2; 0 0 0 0]); testCase.verify_obj(expected_mag_str, swobj.mag_str); end + function test_fourier_multiatom_nExt_nMagAtom_spins(testCase) + % Test where there are the same number of spins provided as in + % the unit cell. Note result is the same as helical with + % complex spins or S=[0 1; 1 0; 0 0] + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0.0], 'S', 2); + k = [0 1/2 0]; + nExt = [int32(1) int32(2) int32(1)]; + swobj.genmagstr('mode', 'fourier', 'S', [-1i 1; 1 1i; 0 0], ... + 'nExt', nExt, 'k', k); + expected_mag_str = struct( ... + 'k', k', ... + 'nExt', nExt, ... + 'F', [-1i 2 1i -2; 1 2i -1 -2i; 0 0 0 0]); + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end function test_helical_multiatom_nExt_nMagAtom_spins(testCase) % Test where there are the same number of spins provided as in % the unit cell @@ -176,8 +201,8 @@ function test_helical_multiatom_multik_multin(testCase) expected_mag_str = testCase.default_mag_str; expected_mag_str.k = k'; expected_mag_str.F = cat(3, ... - [1 1-sqrt(3)*i; 1i sqrt(3)+1i; 0 0], ... - [1 -2i; 0 0; -1i -2]); + [1 1-sqrt(3)*1i; 1i sqrt(3)+1i; 0 0], ... + [1 -2i; 0 0; -1i -2]); testCase.verify_obj(expected_mag_str, swobj.mag_str); end From 04e8b7b75660868d7272372c3800384b7ad7b2d2 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Wed, 27 Jul 2022 13:14:32 +0100 Subject: [PATCH 13/41] Add tile tests --- .../+unit_tests/unittest_spinw_genmagstr.m | 78 ++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 31c24c22..275032aa 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -176,7 +176,7 @@ function test_helical_multiatom_nExt_nMagExt_spins(testCase) % the supercell swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0.0], 'S', 2); - k = [0 1/5 0]; + k = [0 1/2 0]; nExt = [int32(1) int32(2) int32(1)]; swobj.genmagstr('mode', 'helical', ... 'S', [0 1 0 -1; 1 0 0 0; 0 0 1 0], ... @@ -259,6 +259,82 @@ function test_random_structure_multiatom_and_nExt(testCase) testCase.verify_obj(rmfield(expected_mag_str, 'F'), ... rmfield(mag_str1, 'F')); end + function test_tile_no_init_creates_random(testCase) + % Test tile without initialised structure + % (i.e. size(S,2) < nMagAtom) creates a random structure + swobj1 = copy(testCase.swobj); + swobj1.addatom('r', [0.5 0.5 0], 'S', 1); + % Need to use clean obj or the stored structure is used + swobj2 = copy(swobj1); + nExt = [int32(2) int32(1) int32(1)]; + % Also test if we input 'k' it is set to 0 in final struct + swobj1.genmagstr('mode', 'tile', 'nExt', nExt, 'S', [1; 0; 0], 'k', [1/2 0 0]); + mag_str1 = swobj1.mag_str; + swobj2.genmagstr('mode', 'tile', 'nExt', nExt, 'S', [1; 0; 0]); + mag_str2 = swobj2.mag_str; + % Check structure is random each time - F is different + testCase.verifyNotEqual(mag_str1.F, mag_str2.F); + testCase.verifySize(mag_str1.F, [3 4]); + % Check S has correct magnitudes + testCase.verify_val(vecnorm(swobj1.magstr.S, 2), ones(1, 4)); + % Check other fields + expected_mag_str = testCase.default_mag_str; + expected_mag_str.nExt = nExt; + testCase.verify_obj(rmfield(expected_mag_str, 'F'), ... + rmfield(mag_str1, 'F')); + end + function test_tile_existing_struct_extend_cell(testCase) + % Test that tile and increasing nExt will correctly tile + % initialised structure + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0], 'S', 1); + nExt = [int32(1) int32(2) int32(1)]; + swobj.genmagstr('mode', 'direct', 'S', [1 0; 0 1; 0 0]); + swobj.genmagstr('mode', 'tile', 'nExt', nExt); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.nExt = nExt; + expected_mag_str.F = [1 0 1 0; 0 1 0 1; 0 0 0 0]; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_tile_existing_struct_same_size(testCase) + % Test that tile with nExt same as initialised structure + % does nothing + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0], 'S', 1); + nExt = [int32(1) int32(2) int32(1)]; + S = [1 0 0 -1; 0 1 0 0; 0 0 1 0]; + swobj.genmagstr('mode', 'direct', 'S', S, 'nExt', nExt); + swobj.genmagstr('mode', 'tile', 'nExt', nExt); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.nExt = nExt; + expected_mag_str.F = S; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_tile_input_S_extend_cell(testCase) + % Test that tile and input S less than nExt will correctly tile + % input S + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0], 'S', 1); + nExt = [int32(3) int32(1) int32(1)]; + swobj.genmagstr('mode', 'tile', 'nExt', nExt, ... + 'S', [1 0; 0 1; 0 0]); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.nExt = nExt; + expected_mag_str.F = [1 0 1 0 1 0; 0 1 0 1 0 1; 0 0 0 0 0 0]; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_tile_multik(testCase) + % Test that S is summed over third dimension with tile, and k + % is not needed (is this the behaviour we want?) + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0], 'S', 1); + S = cat(3, [1 0; 0 1; 0 0], [0 1; 0 0; 1 0]); + swobj.genmagstr('mode', 'tile', ... + 'S', S); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.F = sqrt(2)/2*[1 1; 0 1; 1 0]; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end end end \ No newline at end of file From a723f49f8a3900c599070b96b273b6aff746d575 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Wed, 27 Jul 2022 15:30:49 +0100 Subject: [PATCH 14/41] Add rotate tests --- .../+unit_tests/unittest_spinw_genmagstr.m | 67 ++++++++++++++++++- 1 file changed, 64 insertions(+), 3 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 275032aa..2cedf0cd 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -267,8 +267,7 @@ function test_tile_no_init_creates_random(testCase) % Need to use clean obj or the stored structure is used swobj2 = copy(swobj1); nExt = [int32(2) int32(1) int32(1)]; - % Also test if we input 'k' it is set to 0 in final struct - swobj1.genmagstr('mode', 'tile', 'nExt', nExt, 'S', [1; 0; 0], 'k', [1/2 0 0]); + swobj1.genmagstr('mode', 'tile', 'nExt', nExt, 'S', [1; 0; 0]); mag_str1 = swobj1.mag_str; swobj2.genmagstr('mode', 'tile', 'nExt', nExt, 'S', [1; 0; 0]); mag_str2 = swobj2.mag_str; @@ -289,7 +288,8 @@ function test_tile_existing_struct_extend_cell(testCase) swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0], 'S', 1); nExt = [int32(1) int32(2) int32(1)]; - swobj.genmagstr('mode', 'direct', 'S', [1 0; 0 1; 0 0]); + % Also test if we input 'k' it is set to 0 in final struct + swobj.genmagstr('mode', 'direct', 'S', [1 0; 0 1; 0 0], 'k', [1/2 0 0]); swobj.genmagstr('mode', 'tile', 'nExt', nExt); expected_mag_str = testCase.default_mag_str; expected_mag_str.nExt = nExt; @@ -335,6 +335,67 @@ function test_tile_multik(testCase) expected_mag_str.F = sqrt(2)/2*[1 1; 0 1; 1 0]; testCase.verify_obj(expected_mag_str, swobj.mag_str); end + function test_rotate_phi(testCase) + swobj = copy(testCase.swobj); + k = [1/2 0 0]; + % Need to initialise structure before rotating it + swobj.genmagstr('mode', 'direct', 'S', [1; 0; 0], 'k', k); + swobj.genmagstr('mode', 'rotate', 'phi', pi/4); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.F = sqrt(2)/2*[1; 1; 0]; + expected_mag_str.k = k'; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_rotate_phid(testCase) + swobj = copy(testCase.swobj); + k = [1/2 0 0]; + swobj.genmagstr('mode', 'direct', 'S', [1; 0; 0], 'k', k); + swobj.genmagstr('mode', 'rotate', 'phid', 45); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.F = sqrt(2)/2*[1; 1; 0]; + expected_mag_str.k = k'; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_rotate_multiatom_n(testCase) + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0], 'S', 1); + k = [1/2 0 0]; + swobj.genmagstr('mode', 'direct', 'S', [1 0; 0 1; 0 0], 'k', k); + swobj.genmagstr('mode', 'rotate', 'phi', pi/2, 'n', [1 1 0]); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.F = [0.5 0.5; 0.5 0.5; -sqrt(2)/2 sqrt(2)/2]; + expected_mag_str.k = k'; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_rotate_no_phi_collinear(testCase) + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0], 'S', 1); + swobj.genmagstr('mode', 'direct', 'S', [1 -1; 0 0; 0 0]); + swobj.genmagstr('mode', 'rotate', 'n', [0 1 0]); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.F = [0 0; 1 -1; 0 0]; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_rotate_no_phi_coplanar(testCase) + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0], 'S', 1); + swobj.genmagstr('mode', 'direct', 'S', [1 0; 0 1; 0 0]); + swobj.genmagstr('mode', 'rotate', 'n', [0 1 0]); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.F = [1 0; 0 0; 0 -1]; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_rotate_no_phi_incomm(testCase) + swobj_tri = copy(testCase.swobj_tri); + k = [1/3 1/3 0]; + swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... + 'k', k); + swobj_tri.genmagstr('mode', 'rotate', 'n', [1 0 0]); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.F = [0; 1.5i; -1.5]; + expected_mag_str.k = k'; + testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); + end end end \ No newline at end of file From 61f77226ac4f210977df6038d4e8f321aee2cc78 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Wed, 27 Jul 2022 16:17:36 +0100 Subject: [PATCH 15/41] Add custom func test --- .../+unit_tests/unittest_spinw_genmagstr.m | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 2cedf0cd..f2cc5f6e 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -30,6 +30,11 @@ function test_no_magnetic_atoms_raises_error(testCase) @() swobj.genmagstr(), ... 'spinw:genmagstr:NoMagAtom') end + function test_invalid_mode_raises_error(testCase) + testCase.verifyError(... + @() testCase.swobj.genmagstr('mode', 'something'), ... + 'spinw:genmagstr:WrongMode') + end function test_zero_nExt_raises_error(testCase) testCase.verifyError(... @() testCase.swobj.genmagstr('nExt', [0 1 1]), ... @@ -396,6 +401,20 @@ function test_rotate_no_phi_incomm(testCase) expected_mag_str.k = k'; testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); end + function test_func_custom(testCase) + function [S, k, n] = func(S0, x0) + S = [-S0; 0; 0]; + k = x0; + n = x0; + end + swobj = copy(testCase.swobj); + x0 = [1/3 0 0]; + swobj.genmagstr('mode', 'func', 'func', @func, 'x0', x0); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.F = [-1; 0; 0]; + expected_mag_str.k = x0'; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end end end \ No newline at end of file From 8189295719a6f3e9c55013969f1b8315f79a33d0 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Mon, 1 Aug 2022 10:38:29 +0100 Subject: [PATCH 16/41] Add default func test --- +sw_tests/+unit_tests/unittest_spinw_genmagstr.m | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index f2cc5f6e..29ac755a 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -401,6 +401,17 @@ function test_rotate_no_phi_incomm(testCase) expected_mag_str.k = k'; testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); end + function test_func_multiatom_default(testCase) + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0], 'S', 1); + k = [1/3 0 0]; + x0 = [pi/2 -pi/4 0 0 k pi pi/2]; + swobj.genmagstr('mode', 'func', 'x0', x0); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.F = [sqrt(2)/2*(1-i) 0; -sqrt(2)/2*(1+i) 0; 0 1]; + expected_mag_str.k = k'; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end function test_func_custom(testCase) function [S, k, n] = func(S0, x0) S = [-S0; 0; 0]; From 4764b469887fcebcc6e9de8e9b00eb249a4d3168 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Mon, 1 Aug 2022 11:00:01 +0100 Subject: [PATCH 17/41] Add extend and unit tests --- .../+unit_tests/unittest_spinw_genmagstr.m | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 29ac755a..adb86bf5 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -35,6 +35,11 @@ function test_invalid_mode_raises_error(testCase) @() testCase.swobj.genmagstr('mode', 'something'), ... 'spinw:genmagstr:WrongMode') end + function test_invalid_unit_raises_error(testCase) + testCase.verifyError(... + @() testCase.swobj.genmagstr('unit', 'something'), ... + 'spinw:genmagstr:WrongInput') + end function test_zero_nExt_raises_error(testCase) testCase.verifyError(... @() testCase.swobj.genmagstr('nExt', [0 1 1]), ... @@ -106,6 +111,17 @@ function test_helical_tri(testCase) expected_mag_str.F = [1.5; 1.5i; 0]; testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); end + function test_helical_tri_lu_unit(testCase) + swobj_tri = copy(testCase.swobj_tri); + swobj_tri.genmagstr('mode', 'helical', 'S', [0; 1; 0], ... + 'k', [1/3 1/3 0], 'unit', 'lu'); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.k = [1/3; 1/3; 0]; + expected_mag_str.F = [-0.75-1.299038105676658i; ... + 1.299038105676658-0.75i; ... + 0]; + testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); + end function test_fourier_tri(testCase) swobj_tri = copy(testCase.swobj_tri); swobj_tri.genmagstr('mode', 'fourier', 'S', [1; 0; 0], ... @@ -340,6 +356,20 @@ function test_tile_multik(testCase) expected_mag_str.F = sqrt(2)/2*[1 1; 0 1; 1 0]; testCase.verify_obj(expected_mag_str, swobj.mag_str); end + function test_extend_input_S_extend_cell(testCase) + % Test undocumented 'extend' mode does same as tile + % Test that tile and input S less than nExt will correctly tile + % input S + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0], 'S', 1); + nExt = [int32(3) int32(1) int32(1)]; + swobj.genmagstr('mode', 'extend', 'nExt', nExt, ... + 'S', [1 0; 0 1; 0 0]); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.nExt = nExt; + expected_mag_str.F = [1 0 1 0 1 0; 0 1 0 1 0 1; 0 0 0 0 0 0]; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end function test_rotate_phi(testCase) swobj = copy(testCase.swobj); k = [1/2 0 0]; From c8b24549febf67d2cc892e6043e05ac0fe1ea3ba Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Mon, 1 Aug 2022 11:17:10 +0100 Subject: [PATCH 18/41] Remove unreachable code Test for number of magnetic atoms is already performed at beginning of function --- swfiles/@spinw/genmagstr.m | 4 ---- 1 file changed, 4 deletions(-) diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index abd6a355..fc403133 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -284,10 +284,6 @@ function genmagstr(obj, varargin) % number of magnetic atoms in the supercell nMagExt = nMagAtom*prod(nExt); -if nMagAtom==0 - error('spinw:genmagstr:NoMagAtom','There is no magnetic atom in the unit cell with S>0!'); -end - % Create mAtom.Sext matrix. mAtom = sw_extendlattice(nExt, mAtom); From 754455be6c4a40525a2eb5f2c4a6480b87b2f5c8 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Mon, 1 Aug 2022 11:47:31 +0100 Subject: [PATCH 19/41] Add scalar nExt test --- .../+unit_tests/unittest_spinw_genmagstr.m | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index adb86bf5..8f26a4d0 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -102,6 +102,23 @@ function test_direct_multiatom_multik(testCase) expected_mag_str.F = cat(3, F_k, F_k); testCase.verify_obj(expected_mag_str, swobj.mag_str); end + function test_direct_multik_scalar_nExt(testCase) + % Test if a scalar is used for nExt it is treated as a + % tolerance to automatically determine nExt + swobj = copy(testCase.swobj); + S_k = [1 0 1 0 1 0; 0 0 1 1 0 0; 0 0 0 1 1 1]; + S = cat(3, S_k, S_k); + nExt = 0.01; + k = [0 1/3+0.5*nExt 0; 1/2+0.5*nExt 0 0]; + swobj.genmagstr('mode', 'direct', 'S', S, 'k', k, 'nExt', nExt); + F_k = [1 0 sqrt(2)/2 0 sqrt(2)/2 0; ... + 0 0 sqrt(2)/2 sqrt(2)/2 0 0; ... + 0 0 0 sqrt(2)/2 sqrt(2)/2 1]; + expected_mag_str = struct('nExt', [int32(2) int32(3) int32(1)], ... + 'k', k', ... + 'F', cat(3, F_k, F_k)); + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end function test_helical_tri(testCase) swobj_tri = copy(testCase.swobj_tri); swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... From 7a56c94a4ad598e09af32cda07d8ad9ed3df4aae Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Mon, 1 Aug 2022 12:02:32 +0100 Subject: [PATCH 20/41] Add more error tests --- +sw_tests/+unit_tests/unittest_spinw_genmagstr.m | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 8f26a4d0..664ac250 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -45,13 +45,26 @@ function test_zero_nExt_raises_error(testCase) @() testCase.swobj.genmagstr('nExt', [0 1 1]), ... 'spinw:genmagstr:WrongInput') end + function test_incompatible_k_and_n_size_raises_error(testCase) + % This error is currently not triggered on parameter input, as + % both 'k' and 'S' are 'soft' parameters, so is triggered later + testCase.verifyError(... + @() testCase.swobj.genmagstr('n', ones(2, 3)),... + 'spinw:genmagstr:WrongInput') + end function test_direct_wrong_spin_size_raises_error(testCase) testCase.verifyError(... @() testCase.swobj.genmagstr( ... 'mode', 'direct', 'S', [0; 1; 0], 'nExt', [2 1 1]), ... 'spinw:genmagstr:WrongSpinSize') end - function test_helical_wrong_spin_size_warns(testCase) + function test_helical_wrong_number_spin_raises_error(testCase) + testCase.verifyError(... + @() testCase.swobj.genmagstr( ... + 'mode', 'helical', 'S', [0 1; 1 0; 0 0]), ... + 'spinw:genmagstr:WrongNumberSpin') + end + function test_helical_spin_size_incomm_with_nExt_warns(testCase) testCase.verifyWarning(... @() testCase.swobj.genmagstr('mode', 'helical', ... 'S', [1 0; 0 1; 0 0], ... From 7d8d56f529b26534d83b7aaca302957d204ab4c5 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Mon, 1 Aug 2022 15:29:52 +0100 Subject: [PATCH 21/41] Add complex rotate and symbolic tests --- .../+unit_tests/unittest_spinw_genmagstr.m | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 664ac250..b45c30cc 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -421,6 +421,18 @@ function test_rotate_phid(testCase) expected_mag_str.k = k'; testCase.verify_obj(expected_mag_str, swobj.mag_str); end + function test_rotate_complex_phi(testCase) + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0], 'S', 1); + k = [1/2 0 0]; + % Need to initialise structure before rotating it + swobj.genmagstr('mode', 'direct', 'S', [0 1; 1 0; 0 0], 'k', k); + swobj.genmagstr('mode', 'rotate', 'phi', i); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.F = [1 0; 0 -1; 0 0]; + expected_mag_str.k = k'; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end function test_rotate_multiatom_n(testCase) swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0], 'S', 1); @@ -487,5 +499,21 @@ function test_func_custom(testCase) testCase.verify_obj(expected_mag_str, swobj.mag_str); end end - + methods (Test, TestTags = {'Symbolic'}) + function test_func_custom_symbolic(testCase) + function [S, k, n] = func(S0, x0) + S = [-S0; 0; 0]; + k = x0; + n = x0; + end + swobj = copy(testCase.swobj); + swobj.symbolic(true); + x0 = [1/3 0 0]; + swobj.genmagstr('mode', 'func', 'func', @func, 'x0', x0); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.F = sym([-1; 0; 0]); + expected_mag_str.k = sym(x0'); + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + end end \ No newline at end of file From 016254d4b4022df395aa66d5bc6c4b7dc87f39fc Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Tue, 9 Aug 2022 13:28:20 +0100 Subject: [PATCH 22/41] Raise error if invalid S/k are provided, improve random tests --- .../+unit_tests/unittest_spinw_genmagstr.m | 68 +++++++++++++++---- CHANGELOG.rst | 2 + swfiles/@spinw/genmagstr.m | 18 ++++- 3 files changed, 72 insertions(+), 16 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index b45c30cc..50cb0489 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -64,6 +64,23 @@ function test_helical_wrong_number_spin_raises_error(testCase) 'mode', 'helical', 'S', [0 1; 1 0; 0 0]), ... 'spinw:genmagstr:WrongNumberSpin') end + function test_invalid_S_raises_error(testCase) + testCase.verifyError(... + @() testCase.swobj.genmagstr('mode', 'direct', 'S', [1 0 0]), ... + 'spinw:genmagstr:WrongInput') + end + function test_invalid_k_raises_error(testCase) + testCase.verifyError(... + @() testCase.swobj.genmagstr('mode', 'direct', 'k', [1/2; 0; 0]), ... + 'spinw:genmagstr:WrongInput') + end + function test_invalid_S_and_k_raises_error(testCase) + testCase.verifyError(... + @() testCase.swobj.genmagstr('mode', 'helical', ... + 'S', [1 0 0], ... + 'k', [1/2; 0; 0]), ... + 'spinw:genmagstr:WrongInput') + end function test_helical_spin_size_incomm_with_nExt_warns(testCase) testCase.verifyWarning(... @() testCase.swobj.genmagstr('mode', 'helical', ... @@ -266,22 +283,42 @@ function test_random_structure(testCase) % Check structure is random each time - F is different testCase.verifyNotEqual(mag_str1.F, mag_str2.F); testCase.verifyEqual(size(mag_str1.F), size(mag_str2.F)); - % Check S has correct size and magnitude - testCase.verify_val(norm(swobj.magstr.S, 2), 1); + % Check F size and magnitude + testCase.verifySize(mag_str1.F, [3 1]); + testCase.verifyEqual(vecnorm(real(swobj.mag_str.F), 2), 1); + % Check imaginary component of F is perpendicular to default n + testCase.verify_val(dot(imag(swobj.mag_str.F), [0 0 1]), 0); + % Check other fields + expected_mag_str = testCase.default_mag_str; + testCase.verify_obj(rmfield(expected_mag_str, 'F'), ... + rmfield(mag_str1, 'F')); + end + function test_random_structure_no_k_with_n(testCase) + swobj = copy(testCase.swobj); + n = [1 1 1]; + swobj.genmagstr('mode','random', 'n', n); + mag_str1 = swobj.mag_str; + % Check F size and magnitude + testCase.verifySize(mag_str1.F, [3 1]); + testCase.verifyEqual(vecnorm(real(swobj.mag_str.F), 2), 1); + % Check imaginary component of F is perpendicular to n + testCase.verify_val(dot(imag(swobj.mag_str.F), n), 0); % Check other fields expected_mag_str = testCase.default_mag_str; testCase.verify_obj(rmfield(expected_mag_str, 'F'), ... rmfield(mag_str1, 'F')); end - function test_random_structure_k(testCase) + function test_random_structure_k_and_n(testCase) swobj = copy(testCase.swobj); k = [0; 0; 1/4]; - swobj.genmagstr('mode','random', 'k', k'); + n = [1 1 0]; + swobj.genmagstr('mode','random', 'k', k', 'n', n); mag_str1 = swobj.mag_str; - % Check F size + % Check F size and magnitude testCase.verifySize(mag_str1.F, [3 1]); - % Check S has correct size and magnitude - testCase.verify_val(vecnorm(swobj.magstr.S, 2), 1); + testCase.verifyEqual(vecnorm(real(swobj.mag_str.F), 2), 1); + % Check imaginary component of F is perpendicular to n + testCase.verify_val(dot(imag(swobj.mag_str.F), n), 0); % Check other fields expected_mag_str = testCase.default_mag_str; expected_mag_str.k = k; @@ -298,12 +335,15 @@ function test_random_structure_multiatom_and_nExt(testCase) mag_str2 = swobj.mag_str; % Check structure is random each time - F is different testCase.verifyNotEqual(mag_str1.F, mag_str2.F); + % Check F size and magnitude testCase.verifySize(mag_str1.F, [3 16]); - % Check S has correct magnitudes - testCase.verify_val(vecnorm(swobj.magstr.S(:, 1:2:end), 2), ... - ones(1, 8)); - testCase.verify_val(vecnorm(swobj.magstr.S(:, 2:2:end), 2), ... - 2*ones(1, 8)); + testCase.verify_val( ... + vecnorm(real(swobj.mag_str.F(:, 1:2:end)), 2), ones(1, 8)); + testCase.verify_val( ... + vecnorm(real(swobj.mag_str.F(:, 2:2:end)), 2), 2*ones(1, 8)); + % Check imaginary component of F is perpendicular to default n + testCase.verifyEqual( ... + dot(imag(swobj.mag_str.F), repmat([0; 0; 1], 1, 16)), zeros(1, 16)); % Check other fields expected_mag_str = testCase.default_mag_str; expected_mag_str.nExt = nExt; @@ -324,9 +364,9 @@ function test_tile_no_init_creates_random(testCase) mag_str2 = swobj2.mag_str; % Check structure is random each time - F is different testCase.verifyNotEqual(mag_str1.F, mag_str2.F); + % Check F size and magnitude testCase.verifySize(mag_str1.F, [3 4]); - % Check S has correct magnitudes - testCase.verify_val(vecnorm(swobj1.magstr.S, 2), ones(1, 4)); + testCase.verify_val(vecnorm(real(swobj1.mag_str.F), 2), ones(1, 4)); % Check other fields expected_mag_str = testCase.default_mag_str; expected_mag_str.nExt = nExt; diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 83245643..499662aa 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -35,3 +35,5 @@ Bug Fixes for a commensurate structure - Fix ``MATLAB:nonLogicalConditional`` error raised when using multiple k in ``genmagstr`` with ``helical`` mode +- Raise error if invalid shape ``S`` or ``k`` is provided to ``genmagstr``, + previously they would be silently set to zero diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index fc403133..5f42a293 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -50,8 +50,8 @@ function genmagstr(obj, varargin) % % `'mode'` % : Mode that determines how the magnetic structure is generated: -% * `'random'` (reads -) -% generates random zero-k magnetic structure. +% * `'random'` (reads `k`, `n`) +% generates random zero-k magnetic structure. % * `'direct'` (reads `S`, `n`, `k`) % direct input of the magnetic structure using the % parameters of the single-k magnetic structure. @@ -220,6 +220,20 @@ function genmagstr(obj, varargin) param = sw_readparam(inpForm, varargin{:}); +% Error if S or k is provided but is empty. This means it has failed the +% input validation, but hasn't caused an error because soft=True +err_str = []; +if any(strcmp(varargin, 'S')) && isempty(param.S) + err_str = ["S"]; +end +if any(strcmp(varargin, 'k')) && isempty(param.k) + err_str = [err_str "k"]; +end +if length(err_str) > 0 + error('spinw:genmagstr:WrongInput', 'Incorrect input size for ' + ... + join(err_str, ', ')); +end + if isempty(param.k) noK = true; param.k = k0; From 0383fd6c34f528b90f039e766dfdf6feee419f21 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Tue, 9 Aug 2022 16:25:48 +0100 Subject: [PATCH 23/41] Raise error with tile and wrong number of input spins --- .../+unit_tests/unittest_spinw_genmagstr.m | 35 +++++++------------ CHANGELOG.rst | 3 ++ swfiles/@spinw/genmagstr.m | 15 ++++---- 3 files changed, 21 insertions(+), 32 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 50cb0489..4ff5b2e5 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -81,6 +81,18 @@ function test_invalid_S_and_k_raises_error(testCase) 'k', [1/2; 0; 0]), ... 'spinw:genmagstr:WrongInput') end + function test_tile_too_few_S_raises_error(testCase) + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0], 'S', 1); + testCase.verifyError(... + @() swobj.genmagstr('mode', 'tile', 'S', [0; 1; 1]), ... + 'spinw:genmagstr:WrongInput') + end + function test_tile_too_many_S_raises_error(testCase) + testCase.verifyError(... + @() testCase.swobj.genmagstr('mode', 'tile', 'S', [0 0; 1 1; 1 1]), ... + 'spinw:genmagstr:WrongInput') + end function test_helical_spin_size_incomm_with_nExt_warns(testCase) testCase.verifyWarning(... @() testCase.swobj.genmagstr('mode', 'helical', ... @@ -350,29 +362,6 @@ function test_random_structure_multiatom_and_nExt(testCase) testCase.verify_obj(rmfield(expected_mag_str, 'F'), ... rmfield(mag_str1, 'F')); end - function test_tile_no_init_creates_random(testCase) - % Test tile without initialised structure - % (i.e. size(S,2) < nMagAtom) creates a random structure - swobj1 = copy(testCase.swobj); - swobj1.addatom('r', [0.5 0.5 0], 'S', 1); - % Need to use clean obj or the stored structure is used - swobj2 = copy(swobj1); - nExt = [int32(2) int32(1) int32(1)]; - swobj1.genmagstr('mode', 'tile', 'nExt', nExt, 'S', [1; 0; 0]); - mag_str1 = swobj1.mag_str; - swobj2.genmagstr('mode', 'tile', 'nExt', nExt, 'S', [1; 0; 0]); - mag_str2 = swobj2.mag_str; - % Check structure is random each time - F is different - testCase.verifyNotEqual(mag_str1.F, mag_str2.F); - % Check F size and magnitude - testCase.verifySize(mag_str1.F, [3 4]); - testCase.verify_val(vecnorm(real(swobj1.mag_str.F), 2), ones(1, 4)); - % Check other fields - expected_mag_str = testCase.default_mag_str; - expected_mag_str.nExt = nExt; - testCase.verify_obj(rmfield(expected_mag_str, 'F'), ... - rmfield(mag_str1, 'F')); - end function test_tile_existing_struct_extend_cell(testCase) % Test that tile and increasing nExt will correctly tile % initialised structure diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 499662aa..92bd5472 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -37,3 +37,6 @@ Bug Fixes k in ``genmagstr`` with ``helical`` mode - Raise error if invalid shape ``S`` or ``k`` is provided to ``genmagstr``, previously they would be silently set to zero +- Raise error if wrong number of spins ``S`` is provided to ``genmagstr`` in + ``helical`` mode. Previously the structure would be silently initialised + to a random structure. diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index 5f42a293..c9397123 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -52,7 +52,7 @@ function genmagstr(obj, varargin) % : Mode that determines how the magnetic structure is generated: % * `'random'` (reads `k`, `n`) % generates random zero-k magnetic structure. -% * `'direct'` (reads `S`, `n`, `k`) +% * `'direct'` (reads `S`, `k`) % direct input of the magnetic structure using the % parameters of the single-k magnetic structure. % * `'tile'` (reads `S`, `n`, `k`) @@ -313,11 +313,6 @@ function genmagstr(obj, varargin) ' to be equal to the number of k-vectors!']) end -% if the magnetic structure is not initialized start with a random real one -if strcmp(param.mode,'tile') && (nMagAtom > size(param.S,2)) - param.mode = 'random'; -end - % convert input into symbolic variables if obj.symb k = sym(k); @@ -337,9 +332,11 @@ function genmagstr(obj, varargin) % cells defined in obj % -the number of spins stored in obj is not equal to the number % of spins in the final structure - if any(double(obj.mag_str.nExt) - double(param.nExt)) || (size(param.S,2) ~= nMagExt) - S = param.S(:,1:nMagAtom,:); - S = repmat(S,[1 prod(nExt) 1]); + if nMagAtom ~= size(param.S,2) && nMagExt ~= size(param.S,2) + error('spinw:genmagstr:WrongInput', ['Incorrect input size for S, ' ... + 'S must be provided for each magnetic atom']); + elseif any(double(obj.mag_str.nExt) - double(param.nExt)) || (size(param.S,2) ~= nMagExt) + S = repmat(param.S,[1 prod(nExt) 1]); else S = param.S; end From ce776b93d1c459a5218d158beed0971f3d24ad9e Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Thu, 11 Aug 2022 11:33:25 +0100 Subject: [PATCH 24/41] Improve input documentation --- .../+unit_tests/unittest_spinw_genmagstr.m | 32 +++++++++---------- swfiles/@spinw/genmagstr.m | 19 +++++++---- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 4ff5b2e5..31eed75c 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -297,7 +297,7 @@ function test_random_structure(testCase) testCase.verifyEqual(size(mag_str1.F), size(mag_str2.F)); % Check F size and magnitude testCase.verifySize(mag_str1.F, [3 1]); - testCase.verifyEqual(vecnorm(real(swobj.mag_str.F), 2), 1); + testCase.verify_val(vecnorm(real(swobj.mag_str.F), 2), 1); % Check imaginary component of F is perpendicular to default n testCase.verify_val(dot(imag(swobj.mag_str.F), [0 0 1]), 0); % Check other fields @@ -305,21 +305,6 @@ function test_random_structure(testCase) testCase.verify_obj(rmfield(expected_mag_str, 'F'), ... rmfield(mag_str1, 'F')); end - function test_random_structure_no_k_with_n(testCase) - swobj = copy(testCase.swobj); - n = [1 1 1]; - swobj.genmagstr('mode','random', 'n', n); - mag_str1 = swobj.mag_str; - % Check F size and magnitude - testCase.verifySize(mag_str1.F, [3 1]); - testCase.verifyEqual(vecnorm(real(swobj.mag_str.F), 2), 1); - % Check imaginary component of F is perpendicular to n - testCase.verify_val(dot(imag(swobj.mag_str.F), n), 0); - % Check other fields - expected_mag_str = testCase.default_mag_str; - testCase.verify_obj(rmfield(expected_mag_str, 'F'), ... - rmfield(mag_str1, 'F')); - end function test_random_structure_k_and_n(testCase) swobj = copy(testCase.swobj); k = [0; 0; 1/4]; @@ -328,7 +313,7 @@ function test_random_structure_k_and_n(testCase) mag_str1 = swobj.mag_str; % Check F size and magnitude testCase.verifySize(mag_str1.F, [3 1]); - testCase.verifyEqual(vecnorm(real(swobj.mag_str.F), 2), 1); + testCase.verify_val(vecnorm(real(swobj.mag_str.F), 2), 1); % Check imaginary component of F is perpendicular to n testCase.verify_val(dot(imag(swobj.mag_str.F), n), 0); % Check other fields @@ -415,6 +400,19 @@ function test_tile_multik(testCase) expected_mag_str.F = sqrt(2)/2*[1 1; 0 1; 1 0]; testCase.verify_obj(expected_mag_str, swobj.mag_str); end + function test_tile_multik_provided_k_set_to_zero(testCase) + % Test that S is summed over third dimension with tile, and if + % k is provided, it is set to zero anyway + swobj = copy(testCase.swobj); + swobj.addatom('r', [0.5 0.5 0], 'S', 1); + S = cat(3, [1 0; 0 1; 0 0], [0 1; 0 0; 1 0]); + k = 0.5*ones(size(S, 3), 3); + swobj.genmagstr('mode', 'tile', ... + 'S', S, 'k', k); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.F = sqrt(2)/2*[1 1; 0 1; 1 0]; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end function test_extend_input_S_extend_cell(testCase) % Test undocumented 'extend' mode does same as tile % Test that tile and input S less than nExt will correctly tile diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index c9397123..b5d87674 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -50,12 +50,17 @@ function genmagstr(obj, varargin) % % `'mode'` % : Mode that determines how the magnetic structure is generated: -% * `'random'` (reads `k`, `n`) -% generates random zero-k magnetic structure. -% * `'direct'` (reads `S`, `k`) +% * `'random'` (optionally reads `k`, `n`, `nExt`) +% generates a random structure in the structural cell if no +% other arguments are specified here or previously in this spinw +% object. If `nExt` is specified all spins in the supercell are +% randomised. If `k` (and optionally `n`) is specified the +% structure is then treated as incommensurate (similarly to the +% `'helical'` mode but with randomized spins in the first cell). +% * `'direct'` (reads `S`, optionally reads `k`, `nExt`) % direct input of the magnetic structure using the % parameters of the single-k magnetic structure. -% * `'tile'` (reads `S`, `n`, `k`) +% * `'tile'` (reads `S`, optionally reads `nExt`) % Simply extends the existing or input structure % (`S`) into a magnetic supercell by replicating it. % If no structure is stored in the [spinw] object a random @@ -67,7 +72,7 @@ function genmagstr(obj, varargin) % Magnetic ordering wavevector `k` will be set to zero. To % generate structure with non-zero k, use the `'helical'` or % `'direct'` option. -% * `'helical'` (reads `S`, `n`, `k`, `r0`) +% * `'helical'` (reads `S`, optionally reads `n`, `k`, `r0`, `nExt`, `epsilon`) % generates helical structure in a single cell or in a % supercell. In contrary to the `'extend'` option the % magnetic structure is not generated by replication but @@ -80,7 +85,7 @@ function genmagstr(obj, varargin) % cell. In the first case $r$ denotes the atomic % positions, while for the second case $r$ denotes the % position of the origin of the cell. -% * `'rotate'` (reads `S`, `n`, `k`) +% * `'rotate'` (optionally reads `S`, `phi`, `phid`, `n`, `nExt`) % uniform rotation of all magnetic moments with a % `phi` angle around the given `n` vector. If % `phi=0`, all moments are rotated so, that the first @@ -102,7 +107,7 @@ function genmagstr(obj, varargin) % vector. The default value for `func` is `@gm_spherical3d`. For planar % magnetic structure use `@gm_planar`. Only `func` and `x` % have to be defined for this mode. -% * `'fourier'` (reads `S`, `n`, `k`, `r0`) +% * `'fourier'` (reads `S`, optionally reads `k`, `r0`, `nExt`, `epsilon`) % same as `'helical'`, except the `S` option is taken as the % Fourier components, thus if it contains real numbers, it will % generate a sinusoidally modulated structure instead of From f6add2dd8d369d627ca555a6159ac2706433b06b Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Thu, 11 Aug 2022 13:04:23 +0100 Subject: [PATCH 25/41] Error if complex S is provided in helical mode --- .../+unit_tests/unittest_spinw_genmagstr.m | 20 +++++++++++++++++-- CHANGELOG.rst | 3 +++ swfiles/@spinw/genmagstr.m | 5 +++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 31eed75c..9aa0b943 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -93,6 +93,11 @@ function test_tile_too_many_S_raises_error(testCase) @() testCase.swobj.genmagstr('mode', 'tile', 'S', [0 0; 1 1; 1 1]), ... 'spinw:genmagstr:WrongInput') end + function test_helical_with_complex_S_raises_error(testCase) + testCase.verifyError(... + @() testCase.swobj.genmagstr('mode', 'helical', 'S', [1.5; 0; 1.5i]), ... + 'spinw:genmagstr:WrongInput') + end function test_helical_spin_size_incomm_with_nExt_warns(testCase) testCase.verifyWarning(... @() testCase.swobj.genmagstr('mode', 'helical', ... @@ -163,13 +168,24 @@ function test_direct_multik_scalar_nExt(testCase) end function test_helical_tri(testCase) swobj_tri = copy(testCase.swobj_tri); + k = [1/3 1/3 0]; swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... - 'k', [1/3 1/3 0]); + 'k', k); expected_mag_str = testCase.default_mag_str; - expected_mag_str.k = [1/3; 1/3; 0]; + expected_mag_str.k = k'; expected_mag_str.F = [1.5; 1.5i; 0]; testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); end + function test_helical_tri_n(testCase) + swobj_tri = copy(testCase.swobj_tri); + k = [1/3 1/3 0]; + swobj_tri.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... + 'k', k, 'n', [0 1 0]); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.k = k'; + expected_mag_str.F = [1.5; 0; -1.5i]; + testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); + end function test_helical_tri_lu_unit(testCase) swobj_tri = copy(testCase.swobj_tri); swobj_tri.genmagstr('mode', 'helical', 'S', [0; 1; 0], ... diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 92bd5472..f48cb8fe 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -40,3 +40,6 @@ Bug Fixes - Raise error if wrong number of spins ``S`` is provided to ``genmagstr`` in ``helical`` mode. Previously the structure would be silently initialised to a random structure. +- Raise error if a complex spin ``S`` is provided to ``genmagstr`` in + ``helical`` mode. Previously this meant it would silently ignore the + ``n`` option, and behave exactly like ``fourier`` mode. diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index b5d87674..a79096b1 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -256,6 +256,11 @@ function genmagstr(obj, varargin) % input type for S, check whether it is complex type cmplxS = ~isreal(param.S); +if strcmpi(param.mode, 'helical') && cmplxS + error('spinw:genmagstr:WrongInput', ... + ['S must be real for helical mode. To specify complex basis ' ... + 'vectors directly use fourier mode.']) +end switch lower(param.unit) case 'lu' From b5bbb72b9c29c4731dadfe5bb15997cbce03bef4 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Thu, 11 Aug 2022 13:24:47 +0100 Subject: [PATCH 26/41] Raise error if rotate mode is used without initialisation --- +sw_tests/+unit_tests/unittest_spinw_genmagstr.m | 5 +++++ CHANGELOG.rst | 2 ++ swfiles/@spinw/genmagstr.m | 5 +++++ 3 files changed, 12 insertions(+) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 9aa0b943..68742504 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -98,6 +98,11 @@ function test_helical_with_complex_S_raises_error(testCase) @() testCase.swobj.genmagstr('mode', 'helical', 'S', [1.5; 0; 1.5i]), ... 'spinw:genmagstr:WrongInput') end + function test_rotate_without_magnetic_structure_raises_error(testCase) + testCase.verifyError(... + @() testCase.swobj.genmagstr('mode', 'rotate'), ... + 'spinw:genmagstr:WrongInput') + end function test_helical_spin_size_incomm_with_nExt_warns(testCase) testCase.verifyWarning(... @() testCase.swobj.genmagstr('mode', 'helical', ... diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f48cb8fe..fa3757ad 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -43,3 +43,5 @@ Bug Fixes - Raise error if a complex spin ``S`` is provided to ``genmagstr`` in ``helical`` mode. Previously this meant it would silently ignore the ``n`` option, and behave exactly like ``fourier`` mode. +- Raise error if ``rotate`` mode is used without first initialising + a magnetic structure diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index a79096b1..d1af72ce 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -239,6 +239,11 @@ function genmagstr(obj, varargin) join(err_str, ', ')); end +if strcmpi(param.mode, 'rotate') && isempty(obj.mag_str.F) + error('spinw:genmagstr:WrongInput', ['rotate mode requires a magnetic ' ... + 'structure to be defined with another mode first']) +end + if isempty(param.k) noK = true; param.k = k0; From 1f2677ad958966556320d84d5090215ff6fcc878 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Thu, 11 Aug 2022 13:25:51 +0100 Subject: [PATCH 27/41] Emit deprecation warning if extend mode is used --- +sw_tests/+unit_tests/unittest_spinw_genmagstr.m | 8 +++++--- CHANGELOG.rst | 2 ++ swfiles/@spinw/genmagstr.m | 2 ++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 68742504..af3536c4 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -434,15 +434,17 @@ function test_tile_multik_provided_k_set_to_zero(testCase) expected_mag_str.F = sqrt(2)/2*[1 1; 0 1; 1 0]; testCase.verify_obj(expected_mag_str, swobj.mag_str); end - function test_extend_input_S_extend_cell(testCase) + function test_extend_mode_input_S_extend_cell_and_warns(testCase) % Test undocumented 'extend' mode does same as tile % Test that tile and input S less than nExt will correctly tile % input S swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0], 'S', 1); nExt = [int32(3) int32(1) int32(1)]; - swobj.genmagstr('mode', 'extend', 'nExt', nExt, ... - 'S', [1 0; 0 1; 0 0]); + testCase.verifyWarning(... + @() swobj.genmagstr('mode', 'extend', 'nExt', nExt, ... + 'S', [1 0; 0 1; 0 0]), ... + 'spinw:genmagstr:DeprecationWarning'); expected_mag_str = testCase.default_mag_str; expected_mag_str.nExt = nExt; expected_mag_str.F = [1 0 1 0 1 0; 0 1 0 1 0 1; 0 0 0 0 0 0]; diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fa3757ad..63310fe3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -45,3 +45,5 @@ Bug Fixes ``n`` option, and behave exactly like ``fourier`` mode. - Raise error if ``rotate`` mode is used without first initialising a magnetic structure +- Emit deprecation warning if the undocumented ``extend`` mode is used + in ``genmagstr`` diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index d1af72ce..c29e6677 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -256,6 +256,8 @@ function genmagstr(obj, varargin) end if strcmp(param.mode,'extend') + warning('spinw:genmagstr:DeprecationWarning',... + 'extend mode is deprecated, please use tile mode instead'); param.mode = 'tile'; end From f6d86c01ed46939cdd136bbca4d0d439823455d5 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Thu, 11 Aug 2022 13:36:41 +0100 Subject: [PATCH 28/41] Error if n and S parallel in rotate mode with no phi --- +sw_tests/+unit_tests/unittest_spinw_genmagstr.m | 7 +++++++ CHANGELOG.rst | 3 +++ swfiles/@spinw/genmagstr.m | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index af3536c4..29a77e94 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -103,6 +103,13 @@ function test_rotate_without_magnetic_structure_raises_error(testCase) @() testCase.swobj.genmagstr('mode', 'rotate'), ... 'spinw:genmagstr:WrongInput') end + function test_rotate_first_spin_parallel_to_n_raises_error(testCase) + swobj = copy(testCase.swobj); + swobj.genmagstr('mode', 'direct', 'S', [0; 0; 1]); + testCase.verifyError(... + @() swobj.genmagstr('mode', 'rotate'), ... + 'spinw:genmagstr:InvalidRotation') + end function test_helical_spin_size_incomm_with_nExt_warns(testCase) testCase.verifyWarning(... @() testCase.swobj.genmagstr('mode', 'helical', ... diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 63310fe3..03591142 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -47,3 +47,6 @@ Bug Fixes a magnetic structure - Emit deprecation warning if the undocumented ``extend`` mode is used in ``genmagstr`` +- Raise error if the first spin is parallel to ``n`` and no rotation + angle is provided in ``rotate`` mode in ``genmagstr``. Previously + this would silently result in ``NaN`` diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index c29e6677..1b387bc9 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -459,6 +459,11 @@ function genmagstr(obj, varargin) % Axis of rotation defined by the spin direction nRot = cross(n,S1); + if all(nRot == 0) + error('spinw:genmagstr:InvalidRotation', ... + ['Cannot automatically determine rotation as the ' ... + 'first spin is parallel to n']); + end % Angle of rotation. phi = -atan2(norm(cross(S1,n)),dot(S1,n)); else From 13ac1522325ae9675287e66a78a18b43da32b0eb Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Thu, 11 Aug 2022 13:47:33 +0100 Subject: [PATCH 29/41] Raise error if imaginary phi is used in rotate mode --- .../+unit_tests/unittest_spinw_genmagstr.m | 19 +++++++------------ CHANGELOG.rst | 2 ++ swfiles/@spinw/genmagstr.m | 8 ++++---- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 29a77e94..ad17f228 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -110,6 +110,13 @@ function test_rotate_first_spin_parallel_to_n_raises_error(testCase) @() swobj.genmagstr('mode', 'rotate'), ... 'spinw:genmagstr:InvalidRotation') end + function test_rotate_complex_phi_raises_error(testCase) + swobj = copy(testCase.swobj); + swobj.genmagstr('mode', 'direct', 'S', [0; 0; 1]); + testCase.verifyError(... + @() swobj.genmagstr('mode', 'rotate', 'phi', i), ... + 'spinw:genmagstr:ComplexPhi') + end function test_helical_spin_size_incomm_with_nExt_warns(testCase) testCase.verifyWarning(... @() testCase.swobj.genmagstr('mode', 'helical', ... @@ -478,18 +485,6 @@ function test_rotate_phid(testCase) expected_mag_str.k = k'; testCase.verify_obj(expected_mag_str, swobj.mag_str); end - function test_rotate_complex_phi(testCase) - swobj = copy(testCase.swobj); - swobj.addatom('r', [0.5 0.5 0], 'S', 1); - k = [1/2 0 0]; - % Need to initialise structure before rotating it - swobj.genmagstr('mode', 'direct', 'S', [0 1; 1 0; 0 0], 'k', k); - swobj.genmagstr('mode', 'rotate', 'phi', i); - expected_mag_str = testCase.default_mag_str; - expected_mag_str.F = [1 0; 0 -1; 0 0]; - expected_mag_str.k = k'; - testCase.verify_obj(expected_mag_str, swobj.mag_str); - end function test_rotate_multiatom_n(testCase) swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0], 'S', 1); diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 03591142..844a2a9e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -50,3 +50,5 @@ Bug Fixes - Raise error if the first spin is parallel to ``n`` and no rotation angle is provided in ``rotate`` mode in ``genmagstr``. Previously this would silently result in ``NaN`` +- Raise error if ``phi`` or ``phid`` is not real in ``rotate`` mode in + ``genmagstr``. This was an undocumented feature which has been removed. diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index 1b387bc9..53d5e6ad 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -441,10 +441,10 @@ function genmagstr(obj, varargin) S = param.S; if ~isreal(param.phi) - % rotate the first spin along [100] - S1 = S(:,1)-sum(n*S(:,1))*n'; - S1 = S1/norm(S1); - param.phi = -atan2(cross(n,[1 0 0])*S1,[1 0 0]*S1); + error('spinw:genmagstr:ComplexPhi', ... + ['Phi should be real! If you really intended phi to have ' ... + 'a complex component, this undocumented feature has been ' ... + 'removed. Please contact the spinw developers.']) end if param.phi == 0 From f4d907f5e891c46342585980be5eac1c9289c129 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Thu, 11 Aug 2022 16:42:53 +0100 Subject: [PATCH 30/41] Refactor tests - use TestParameter --- .../+unit_tests/unittest_spinw_genmagstr.m | 131 +++++++----------- 1 file changed, 50 insertions(+), 81 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index ad17f228..4c1da865 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -7,7 +7,40 @@ 'k', [0; 0; 0], ... 'F', [0; 1; 0]); end - + properties (TestParameter) + fm_chain_input_errors = { ... + % varargin, identifier + {{'mode', 'something'}, 'spinw:genmagstr:WrongMode'}; ... + {{'unit', 'something'}, 'spinw:genmagstr:WrongInput'}; ... + % nExt can't be zero + {{'nExt', [0 1 1]}, 'spinw:genmagstr:WrongInput'}; ... + % n must have dimensions nK, 3 + {{'n', ones(2, 3), 'k', [0 0 1/2]}, 'sw_readparam:ParameterSizeMismatch'}; ... + % S with direct must have same number of spins as atoms in nExt supercell + {{'mode', 'direct', 'S', [0; 1; 0], 'nExt', [2 1 1]}, 'spinw:genmagstr:WrongSpinSize'}; ... + % S with helical must have 1 spin, or same number of spins as + % atoms in unit or supercell + {{'mode', 'helical', 'S', [0 1; 1 0; 0 0]}, 'spinw:genmagstr:WrongNumberSpin'}; ... + {{'mode', 'direct', 'S', [1 0 0]}, 'spinw:genmagstr:WrongInput'}; ... + {{'mode', 'direct', 'k', [1/2; 0; 0]}, 'spinw:genmagstr:WrongInput'}; ... + {{'mode', 'direct', 'S', [1 0 0], 'k', [1/2; 0; 0]}, 'spinw:genmagstr:WrongInput'}; ... + {{'mode', 'tile', 'S', [0 0; 1 1; 1 1]}, 'spinw:genmagstr:WrongInput'}; ... + % S with helical must be real + {{'mode', 'helical', 'S', [1.5; 0; 1.5i]}, 'spinw:genmagstr:WrongInput'}; ... + % Rotate mode must first initialise a magnetic structure + {{'mode', 'rotate'}, 'spinw:genmagstr:WrongInput'} + }; + rotate_input_errors = { ... + % varargin, identifier + % rotation angle must be real (previously phi=i had special + % behaviour) + {{'mode', 'rotate', 'phi', i}, 'spinw:genmagstr:ComplexPhi'} + % If no angle is supplied to rotate, the rotation axis is set + % orthogonal to both the first spin and n. If the first spin + % and n are parallel, this should therefore cause an error + {{'mode', 'rotate'}, 'spinw:genmagstr:InvalidRotation'}; ... + }; + end methods (TestMethodSetup) function setup_chain_model(testCase) % Create a simple FM 1D chain model @@ -24,62 +57,27 @@ function setup_tri_model(testCase) end methods (Test) - function test_no_magnetic_atoms_raises_error(testCase) - swobj = spinw; - testCase.verifyError(... - @() swobj.genmagstr(), ... - 'spinw:genmagstr:NoMagAtom') - end - function test_invalid_mode_raises_error(testCase) - testCase.verifyError(... - @() testCase.swobj.genmagstr('mode', 'something'), ... - 'spinw:genmagstr:WrongMode') - end - function test_invalid_unit_raises_error(testCase) - testCase.verifyError(... - @() testCase.swobj.genmagstr('unit', 'something'), ... - 'spinw:genmagstr:WrongInput') - end - function test_zero_nExt_raises_error(testCase) + function test_invalid_fm_chain_input_raises_error(testCase, fm_chain_input_errors) + varargin = fm_chain_input_errors{1}; + identifier = fm_chain_input_errors{2}; testCase.verifyError(... - @() testCase.swobj.genmagstr('nExt', [0 1 1]), ... - 'spinw:genmagstr:WrongInput') - end - function test_incompatible_k_and_n_size_raises_error(testCase) - % This error is currently not triggered on parameter input, as - % both 'k' and 'S' are 'soft' parameters, so is triggered later - testCase.verifyError(... - @() testCase.swobj.genmagstr('n', ones(2, 3)),... - 'spinw:genmagstr:WrongInput') - end - function test_direct_wrong_spin_size_raises_error(testCase) - testCase.verifyError(... - @() testCase.swobj.genmagstr( ... - 'mode', 'direct', 'S', [0; 1; 0], 'nExt', [2 1 1]), ... - 'spinw:genmagstr:WrongSpinSize') - end - function test_helical_wrong_number_spin_raises_error(testCase) - testCase.verifyError(... - @() testCase.swobj.genmagstr( ... - 'mode', 'helical', 'S', [0 1; 1 0; 0 0]), ... - 'spinw:genmagstr:WrongNumberSpin') + @() testCase.swobj.genmagstr(varargin{:}), ... + identifier) end - function test_invalid_S_raises_error(testCase) - testCase.verifyError(... - @() testCase.swobj.genmagstr('mode', 'direct', 'S', [1 0 0]), ... - 'spinw:genmagstr:WrongInput') - end - function test_invalid_k_raises_error(testCase) + function test_invalid_rotate_input_raises_error(testCase, rotate_input_errors) + varargin = rotate_input_errors{1}; + identifier = rotate_input_errors{2}; + swobj = copy(testCase.swobj); + swobj.genmagstr('mode', 'direct', 'S', [0; 0; 1]); testCase.verifyError(... - @() testCase.swobj.genmagstr('mode', 'direct', 'k', [1/2; 0; 0]), ... - 'spinw:genmagstr:WrongInput') + @() swobj.genmagstr(varargin{:}), ... + identifier) end - function test_invalid_S_and_k_raises_error(testCase) + function test_no_magnetic_atoms_raises_error(testCase) + swobj = spinw; testCase.verifyError(... - @() testCase.swobj.genmagstr('mode', 'helical', ... - 'S', [1 0 0], ... - 'k', [1/2; 0; 0]), ... - 'spinw:genmagstr:WrongInput') + @() swobj.genmagstr(), ... + 'spinw:genmagstr:NoMagAtom') end function test_tile_too_few_S_raises_error(testCase) swobj = copy(testCase.swobj); @@ -88,35 +86,6 @@ function test_tile_too_few_S_raises_error(testCase) @() swobj.genmagstr('mode', 'tile', 'S', [0; 1; 1]), ... 'spinw:genmagstr:WrongInput') end - function test_tile_too_many_S_raises_error(testCase) - testCase.verifyError(... - @() testCase.swobj.genmagstr('mode', 'tile', 'S', [0 0; 1 1; 1 1]), ... - 'spinw:genmagstr:WrongInput') - end - function test_helical_with_complex_S_raises_error(testCase) - testCase.verifyError(... - @() testCase.swobj.genmagstr('mode', 'helical', 'S', [1.5; 0; 1.5i]), ... - 'spinw:genmagstr:WrongInput') - end - function test_rotate_without_magnetic_structure_raises_error(testCase) - testCase.verifyError(... - @() testCase.swobj.genmagstr('mode', 'rotate'), ... - 'spinw:genmagstr:WrongInput') - end - function test_rotate_first_spin_parallel_to_n_raises_error(testCase) - swobj = copy(testCase.swobj); - swobj.genmagstr('mode', 'direct', 'S', [0; 0; 1]); - testCase.verifyError(... - @() swobj.genmagstr('mode', 'rotate'), ... - 'spinw:genmagstr:InvalidRotation') - end - function test_rotate_complex_phi_raises_error(testCase) - swobj = copy(testCase.swobj); - swobj.genmagstr('mode', 'direct', 'S', [0; 0; 1]); - testCase.verifyError(... - @() swobj.genmagstr('mode', 'rotate', 'phi', i), ... - 'spinw:genmagstr:ComplexPhi') - end function test_helical_spin_size_incomm_with_nExt_warns(testCase) testCase.verifyWarning(... @() testCase.swobj.genmagstr('mode', 'helical', ... From 8c2a818590d7ac920970f66d11d0f2d409c25c09 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Fri, 12 Aug 2022 11:47:51 +0100 Subject: [PATCH 31/41] Clarify random mode docstring --- swfiles/@spinw/genmagstr.m | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index 53d5e6ad..fee88edf 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -51,12 +51,13 @@ function genmagstr(obj, varargin) % `'mode'` % : Mode that determines how the magnetic structure is generated: % * `'random'` (optionally reads `k`, `n`, `nExt`) -% generates a random structure in the structural cell if no -% other arguments are specified here or previously in this spinw +% generates a random structure in the structural cell if no other +% arguments are specified here or previously in this spinw % object. If `nExt` is specified all spins in the supercell are -% randomised. If `k` (and optionally `n`) is specified the -% structure is then treated as incommensurate (similarly to the -% `'helical'` mode but with randomized spins in the first cell). +% randomised. If `k` is specified a random helical structure with +% moments perpendicular to `n` (default value: `[0 0 1]`) with +% the specified `k` propagation vector is generated. (`n` is not +% otherwise used). % * `'direct'` (reads `S`, optionally reads `k`, `nExt`) % direct input of the magnetic structure using the % parameters of the single-k magnetic structure. From 9cfdd15492852320267a3251e3be374deaefcaee Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Tue, 16 Aug 2022 11:41:09 +0100 Subject: [PATCH 32/41] Warn if n || S for helical, and add tests --- .../+unit_tests/unittest_spinw_genmagstr.m | 80 ++++++++++++------- CHANGELOG.rst | 2 + swfiles/@spinw/genmagstr.m | 11 +++ 3 files changed, 63 insertions(+), 30 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 4c1da865..06621d02 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -3,7 +3,7 @@ properties swobj = []; swobj_tri = []; - default_mag_str = struct('nExt', ones(1, 3, 'int32'), ... + default_mag_str = struct('nExt', int32([1 1 1]), ... 'k', [0; 0; 0], ... 'F', [0; 1; 0]); end @@ -87,13 +87,28 @@ function test_tile_too_few_S_raises_error(testCase) 'spinw:genmagstr:WrongInput') end function test_helical_spin_size_incomm_with_nExt_warns(testCase) + swobj = copy(testCase.swobj); testCase.verifyWarning(... - @() testCase.swobj.genmagstr('mode', 'helical', ... - 'S', [1 0; 0 1; 0 0], ... - 'k', [1/3 0 0], ... - 'nExt', [2 1 1]), ... + @() swobj.genmagstr('mode', 'helical', ... + 'S', [1 0; 0 1; 0 0], ... + 'k', [1/3 0 0], ... + 'nExt', [2 1 1]), ... 'spinw:genmagstr:UCExtNonSuff') end + function test_helical_any_S_parallel_to_n_warns(testCase) + swobj_tri = copy(testCase.swobj_tri); + k = [1/3 0 0]; + testCase.verifyWarning(... + @() swobj_tri.genmagstr('mode', 'helical', ... + 'S', [0; 1; 1], ... + 'k', k), ... + 'spinw:genmagstr:SnParallel') + expected_mag_str = struct( ... + 'nExt', int32([1 1 1]), ... + 'k', k', ... + 'F', [-sqrt(9/8)*1i; sqrt(9/8); sqrt(9/8)]); + testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); + end function test_direct_fm_chain(testCase) swobj = copy(testCase.swobj); swobj.genmagstr('mode', 'direct', 'k', [0 0 0], ... @@ -112,10 +127,10 @@ function test_direct_multiatom_nExt(testCase) S = [1 0 1 -1; ... 1 1 0 0; ... 0 -1 0 0]; - nExt = [int32(2) int32(1) int32(1)]; + nExt = [2 1 1]; k = [0 1/3 0]; swobj.genmagstr('mode', 'direct', 'S', S, 'nExt', nExt, 'k', k); - expected_mag_str = struct('nExt', nExt, ... + expected_mag_str = struct('nExt', int32(nExt), ... 'k', k', ... 'F', [sqrt(2)/2 0 1 -2; ... sqrt(2)/2 sqrt(2) 0 0; ... @@ -149,7 +164,7 @@ function test_direct_multik_scalar_nExt(testCase) F_k = [1 0 sqrt(2)/2 0 sqrt(2)/2 0; ... 0 0 sqrt(2)/2 sqrt(2)/2 0 0; ... 0 0 0 sqrt(2)/2 sqrt(2)/2 1]; - expected_mag_str = struct('nExt', [int32(2) int32(3) int32(1)], ... + expected_mag_str = struct('nExt', int32([2 3 1]), ... 'k', k', ... 'F', cat(3, F_k, F_k)); testCase.verify_obj(expected_mag_str, swobj.mag_str); @@ -199,7 +214,7 @@ function test_helical_multiatom_nExt_1spin(testCase) swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0.0], 'S', 2); k = [0 0 1/2]; - nExt = [int32(1) int32(1) int32(2)]; + nExt = int32([1 1 2]); swobj.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... 'nExt', nExt, 'k', k); expected_mag_str = struct(... @@ -215,7 +230,7 @@ function test_helical_multiatom_nExt_1spin_r0(testCase) swobj.addatom('r', [0.5 0.5 0.5],'S', 1); swobj.addatom('r', [0 0 0], 'S', 2); k = [0 0 1/2]; - nExt = [int32(1) int32(1) int32(2)]; + nExt = int32([1 1 2]); swobj.genmagstr('mode', 'helical', 'S', [1; 0; 0], ... 'nExt', nExt, 'k', k, 'r0', false); expected_mag_str = struct(... @@ -231,7 +246,7 @@ function test_fourier_multiatom_nExt_nMagAtom_spins(testCase) swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0.0], 'S', 2); k = [0 1/2 0]; - nExt = [int32(1) int32(2) int32(1)]; + nExt = int32([1 2 1]); swobj.genmagstr('mode', 'fourier', 'S', [-1i 1; 1 1i; 0 0], ... 'nExt', nExt, 'k', k); expected_mag_str = struct( ... @@ -246,7 +261,7 @@ function test_helical_multiatom_nExt_nMagAtom_spins(testCase) swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0.0], 'S', 2); k = [0 1/2 0]; - nExt = [int32(1) int32(2) int32(1)]; + nExt = int32([1 2 1]); swobj.genmagstr('mode', 'helical', 'S', [0 1; 1 0; 0 0], ... 'nExt', nExt, 'k', k); expected_mag_str = struct( ... @@ -261,10 +276,13 @@ function test_helical_multiatom_nExt_nMagExt_spins(testCase) swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0.0], 'S', 2); k = [0 1/2 0]; - nExt = [int32(1) int32(2) int32(1)]; - swobj.genmagstr('mode', 'helical', ... - 'S', [0 1 0 -1; 1 0 0 0; 0 0 1 0], ... - 'nExt', nExt, 'k', k); + nExt = int32([1 2 1]); + testCase.verifyWarning(... + @() swobj.genmagstr( ... + 'mode', 'helical', ... + 'S', [0 1 0 -1; 1 0 0 0; 0 0 1 0], ... + 'nExt', nExt, 'k', k), ... + 'spinw:genmagstr:SnParallel'); expected_mag_str = struct(... 'k', k', ... 'nExt', nExt, ... @@ -274,20 +292,22 @@ function test_helical_multiatom_nExt_nMagExt_spins(testCase) function test_helical_multiatom_multik_multin(testCase) swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0.5], 'S', 2); - S_k = [1; 0; 0]; - S = cat(3, S_k, S_k); + S = cat(3, [1; 0; 0], [0; 0; 1]); k = [0 1/3 0; 1/2 0 0]; n = [0 0 1; 0 1 0]; - swobj.genmagstr('mode', 'helical', 'S', S, 'k', k, 'n', n); - F_k = [sqrt(2)/2 0; ... - sqrt(2)/2 sqrt(2); ... - 0 -sqrt(2)]; + % Ensure warning is not emitted as there are no S parallel to + % n within a single k + testCase.verifyWarningFree(... + @() swobj.genmagstr('mode', 'helical', ... + 'S', S, ... + 'k', k, ... + 'n', n), ... + 'spinw:genmagstr:SnParallel') expected_mag_str = testCase.default_mag_str; expected_mag_str.k = k'; expected_mag_str.F = cat(3, ... - [1 1-sqrt(3)*1i; 1i sqrt(3)+1i; 0 0], ... - [1 -2i; 0 0; -1i -2]); - + [1 1-sqrt(3)*1i; 1i sqrt(3)+1i; 0 0], ... + [1i 2; 0 0; 1 -2i]); testCase.verify_obj(expected_mag_str, swobj.mag_str); end function test_random_structure(testCase) @@ -329,7 +349,7 @@ function test_random_structure_k_and_n(testCase) function test_random_structure_multiatom_and_nExt(testCase) swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0], 'S', 2); - nExt = 2*ones(1, 3, 'int32'); + nExt = int32([2 2 2]); swobj.genmagstr('mode', 'random', 'nExt', nExt); mag_str1 = swobj.mag_str; swobj.genmagstr('mode', 'random', 'nExt', nExt); @@ -356,7 +376,7 @@ function test_tile_existing_struct_extend_cell(testCase) % initialised structure swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0], 'S', 1); - nExt = [int32(1) int32(2) int32(1)]; + nExt = int32([1 2 1]); % Also test if we input 'k' it is set to 0 in final struct swobj.genmagstr('mode', 'direct', 'S', [1 0; 0 1; 0 0], 'k', [1/2 0 0]); swobj.genmagstr('mode', 'tile', 'nExt', nExt); @@ -370,7 +390,7 @@ function test_tile_existing_struct_same_size(testCase) % does nothing swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0], 'S', 1); - nExt = [int32(1) int32(2) int32(1)]; + nExt = int32([1 2 1]); S = [1 0 0 -1; 0 1 0 0; 0 0 1 0]; swobj.genmagstr('mode', 'direct', 'S', S, 'nExt', nExt); swobj.genmagstr('mode', 'tile', 'nExt', nExt); @@ -384,7 +404,7 @@ function test_tile_input_S_extend_cell(testCase) % input S swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0], 'S', 1); - nExt = [int32(3) int32(1) int32(1)]; + nExt = int32([3 1 1]); swobj.genmagstr('mode', 'tile', 'nExt', nExt, ... 'S', [1 0; 0 1; 0 0]); expected_mag_str = testCase.default_mag_str; @@ -423,7 +443,7 @@ function test_extend_mode_input_S_extend_cell_and_warns(testCase) % input S swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0], 'S', 1); - nExt = [int32(3) int32(1) int32(1)]; + nExt = int32([3 1 1]); testCase.verifyWarning(... @() swobj.genmagstr('mode', 'extend', 'nExt', nExt, ... 'S', [1 0; 0 1; 0 0]), ... diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 844a2a9e..0307520d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -52,3 +52,5 @@ Bug Fixes this would silently result in ``NaN`` - Raise error if ``phi`` or ``phid`` is not real in ``rotate`` mode in ``genmagstr``. This was an undocumented feature which has been removed. +- Emit warning that the spin amplitude will be moderated if components + of ``S`` are parallel to ``n`` in ``helical`` mode diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index fee88edf..8d650968 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -374,6 +374,17 @@ function genmagstr(obj, varargin) S = S + 1i*cross(repmat(permute(n,[2 3 1]),[1 size(S,2) 1]),S); case {'helical' 'fourier'} + if strcmpi(param.mode, 'helical') + for ik=1:size(k, 1) + Sk = real(param.S(:, :, ik)); + if any(dot(repmat(n(ik, :), size(Sk, 2), 1)', Sk)) + warning('spinw:genmagstr:SnParallel', ... + ['There are spin components parallel to n, the ' ... + 'amplitude of these components will be modulated']); + break; + end + end + end S0 = param.S; % Magnetic ordering wavevector in the extended unit cell. kExt = k.*nExt; From 4bee789b7355d5f582236b81ac0d6f2c6dd10cbb Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Tue, 16 Aug 2022 11:48:39 +0100 Subject: [PATCH 33/41] Small upates from review comments - Specify n is converted to a unit vector in docstring - Remove unreachable n/k size error check --- swfiles/@spinw/genmagstr.m | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index 8d650968..0c20178c 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -138,8 +138,9 @@ function genmagstr(obj, varargin) % % `'n'` % : Normal vector to the spin rotation plane for single-k magnetic -% structures, stored in a 3-element row vector. Default value `[0 0 1]`. The -% coordinate system of the vector is determined by `unit`. +% structures, stored in a 3-element row vector, is automatically +% normalised to a unit vector. Default value `[0 0 1]`. The coordinate +% system of the vector is determined by `unit`. % % `'S'` % : Vector values of the spins (expectation value), dimensions are $[3\times n_{spin} n_K]$. @@ -326,11 +327,6 @@ function genmagstr(obj, varargin) end n = bsxfunsym(@rdivide,param.n,sqrt(sum(param.n.^2,2))); -if size(param.n,1) ~= nK - error('spinw:genmagstr:WrongInput',['The number of normal vectors has'... - ' to be equal to the number of k-vectors!']) -end - % convert input into symbolic variables if obj.symb k = sym(k); From 4e161d9946b1a689a049854c5252e8e865a0f592 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Tue, 16 Aug 2022 13:25:01 +0100 Subject: [PATCH 34/41] Warn if nExt is too large in helical, fourier --- .../+unit_tests/unittest_spinw_genmagstr.m | 44 ++++++++++++++++++- CHANGELOG.rst | 2 + swfiles/@spinw/genmagstr.m | 11 ++++- 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 06621d02..dfaf0d59 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -88,12 +88,52 @@ function test_tile_too_few_S_raises_error(testCase) end function test_helical_spin_size_incomm_with_nExt_warns(testCase) swobj = copy(testCase.swobj); + nExt = [2 1 1]; + k = [1/3 0 0]; testCase.verifyWarning(... @() swobj.genmagstr('mode', 'helical', ... 'S', [1 0; 0 1; 0 0], ... - 'k', [1/3 0 0], ... - 'nExt', [2 1 1]), ... + 'k', k, ... + 'nExt', nExt), ... 'spinw:genmagstr:UCExtNonSuff') + expected_mag_str = struct('nExt', int32(nExt), ... + 'k', k', ... + 'F', [1 -1i; 1i 1; 0 0]); + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_fourier_too_large_nExt_warns(testCase) + swobj = copy(testCase.swobj); + nExt = [6 1 1]; + k = [1/3 0 0]; + testCase.verifyWarning(... + @() swobj.genmagstr('mode', 'fourier', ... + 'S', [1; 1i; 0], ... + 'k', k, ... + 'nExt', nExt), ... + 'spinw:genmagstr:UCExtOver') + F_rep = [ 1 -0.5-1i*sqrt(3)/2 -0.5+1i*sqrt(3)/2; ... + 1i sqrt(3)/2-0.5i -sqrt(3)/2-0.5i; ... + 0 0 0]; + expected_mag_str = struct('nExt', int32(nExt), ... + 'k', k', ... + 'F', cat(2, F_rep, F_rep)); + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end + function test_fourier_nExt_wrong_direction_warns(testCase) + swobj = copy(testCase.swobj); + nExt = [2 1 2]; + k = [1/2 0 0]; + testCase.verifyWarning(... + @() swobj.genmagstr('mode', 'fourier', ... + 'S', [1; 1i; 0], ... + 'k', k, ... + 'nExt', nExt), ... + 'spinw:genmagstr:UCExtOver') + F_rep = [1 -1; 1i -1i; 0 0]; + expected_mag_str = struct('nExt', int32(nExt), ... + 'k', k', ... + 'F', cat(2, F_rep, F_rep)); + testCase.verify_obj(expected_mag_str, swobj.mag_str); end function test_helical_any_S_parallel_to_n_warns(testCase) swobj_tri = copy(testCase.swobj_tri); diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0307520d..bb41c0aa 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -54,3 +54,5 @@ Bug Fixes ``genmagstr``. This was an undocumented feature which has been removed. - Emit warning that the spin amplitude will be moderated if components of ``S`` are parallel to ``n`` in ``helical`` mode +- Emit warning if ``nExt`` is unnecessarily large compared to ``k`` in + ``helical`` and ``fourier`` modes diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index 0c20178c..235a746f 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -384,11 +384,18 @@ function genmagstr(obj, varargin) S0 = param.S; % Magnetic ordering wavevector in the extended unit cell. kExt = k.*nExt; - % Warns about the non sufficient extension of the unit cell. - % we substitute random values for symbolic km + % Substitute random values for symbolic km skExt = sw_sub1(kExt,'rand'); if any(abs(skExt(:)-round(skExt(:)))>param.epsilon) && prod(nExt) > 1 + % Warns about the non sufficient extension of the unit cell. warning('spinw:genmagstr:UCExtNonSuff','In the extended unit cell k is still larger than epsilon!'); + else + idx = find(sum(k, 1) == 0); + if any(round(skExt(:)) > 1) || any(nExt(idx) > 1) + % Warns about overextension of the unit cell. + warning('spinw:genmagstr:UCExtOver', ... + 'Unit cell has been extended beyond what is required for the given k') + end end % number of spins in the input nSpin = size(param.S,2); From d8d54f3eba16616ed409a1a5ef300e125620693c Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Wed, 17 Aug 2022 08:54:04 +0100 Subject: [PATCH 35/41] Warn on unused inputs - Also move 'extend' -> 'tile' to the top so tile can be used in unused input checks - Also move valid mode check to in unused warning checks so an invalid mode isn't searched for in the struct --- .../+unit_tests/unittest_spinw_genmagstr.m | 24 +++++++- CHANGELOG.rst | 6 +- swfiles/@spinw/genmagstr.m | 57 ++++++++++++------- 3 files changed, 64 insertions(+), 23 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index dfaf0d59..08cbf67e 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -34,12 +34,21 @@ % varargin, identifier % rotation angle must be real (previously phi=i had special % behaviour) - {{'mode', 'rotate', 'phi', i}, 'spinw:genmagstr:ComplexPhi'} + {{'mode', 'rotate', 'phi', i}, 'spinw:genmagstr:ComplexPhi'}; ... % If no angle is supplied to rotate, the rotation axis is set % orthogonal to both the first spin and n. If the first spin % and n are parallel, this should therefore cause an error {{'mode', 'rotate'}, 'spinw:genmagstr:InvalidRotation'}; ... }; + ignored_inputs = { ... + % arguments + {'mode', 'random', 'S', [1; 0; 0], 'epsilon', 1}, ... + {'n', [0 1 1], 'mode', 'direct', 'S', [1; 0; 0]}, ... + {'mode', 'tile', 'k', [0 0 1/2], 'n', [0 0 1], 'S', [1; 0; 0]}, ... + {'mode', 'helical', 'k', [0 0 1/3], 'S', [1; 0; 0;], 'x0', []}, ... + {'mode', 'func', 'x0', [pi/2 -pi/4 0 0 [0 0 1/3] pi pi/2], 'unit', 'lu'}, ... + {'mode', 'fourier', 'S',[1; i; 0], 'n', [1 0 0]} + }; end methods (TestMethodSetup) function setup_chain_model(testCase) @@ -86,6 +95,19 @@ function test_tile_too_few_S_raises_error(testCase) @() swobj.genmagstr('mode', 'tile', 'S', [0; 1; 1]), ... 'spinw:genmagstr:WrongInput') end + function test_ignored_input_warns(testCase, ignored_inputs) + testCase.verifyWarning(... + @() testCase.swobj.genmagstr(ignored_inputs{:}), ... + 'spinw:genmagstr:UnusedInput') + end + function test_rotate_ignored_input_warns(testCase) + swobj = copy(testCase.swobj); + % Need to initialise structure before rotating it + swobj.genmagstr('mode', 'direct', 'S', [1; 0; 0]); + testCase.verifyWarning(... + @() swobj.genmagstr('mode', 'rotate', 'k', [0 0 1/3]), ... + 'spinw:genmagstr:UnusedInput') + end function test_helical_spin_size_incomm_with_nExt_warns(testCase) swobj = copy(testCase.swobj); nExt = [2 1 1]; diff --git a/CHANGELOG.rst b/CHANGELOG.rst index bb41c0aa..f05eef50 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -53,6 +53,8 @@ Bug Fixes - Raise error if ``phi`` or ``phid`` is not real in ``rotate`` mode in ``genmagstr``. This was an undocumented feature which has been removed. - Emit warning that the spin amplitude will be moderated if components - of ``S`` are parallel to ``n`` in ``helical`` mode + of ``S`` are parallel to ``n`` in ``helical`` mode in ``genmagstr`` - Emit warning if ``nExt`` is unnecessarily large compared to ``k`` in - ``helical`` and ``fourier`` modes + ``helical`` and ``fourier`` modes in ``genmagstr`` +- Emit warning if arguments that will be ignored are passed to a particular + mode in ``genmagstr`` (e.g. ``S`` is passed to ``random``) diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index 235a746f..67e5daf5 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -246,6 +246,32 @@ function genmagstr(obj, varargin) 'structure to be defined with another mode first']) end +if strcmp(param.mode,'extend') + warning('spinw:genmagstr:DeprecationWarning',... + 'extend mode is deprecated, please use tile mode instead'); + param.mode = 'tile'; +end + +mode_args = struct("random", ["k", "n", "nExt", "unit"], ... + "direct", ["S", "k", "nExt", "unit"], ... + "tile", ["S", "nExt", "unit"], ... + "helical", ["S", "n", "k", "r0", "nExt", "epsilon", "unit"], ... + "rotate", ["S", "phi", "phid", "n", "nExt", "unit"], ... + "func", ["func", "x0"], ... + "fourier", ["S", "k", "r0", "nExt", "epsilon", "unit"]); +if ~any(strcmp(fields(mode_args), param.mode)) + error('spinw:genmagstr:WrongMode','Wrong param.mode value!'); +else + input_arg_names = varargin(1:2:end); + input_arg_names(ismember(input_arg_names, "mode")) = []; + unused_args = setdiff(input_arg_names, mode_args.(param.mode)); + if ~isempty(unused_args) + warning('spinw:genmagstr:UnusedInput', ... + 'Input(s) %s will be ignored in %s mode', ... + join(unused_args, ', '), param.mode) + end +end + if isempty(param.k) noK = true; param.k = k0; @@ -257,12 +283,6 @@ function genmagstr(obj, varargin) error('spinw:genmagstr:WrongInput','''nExt'' has to be larger than 0!'); end -if strcmp(param.mode,'extend') - warning('spinw:genmagstr:DeprecationWarning',... - 'extend mode is deprecated, please use tile mode instead'); - param.mode = 'tile'; -end - % input type for S, check whether it is complex type cmplxS = ~isreal(param.S); if strcmpi(param.mode, 'helical') && cmplxS @@ -370,17 +390,6 @@ function genmagstr(obj, varargin) S = S + 1i*cross(repmat(permute(n,[2 3 1]),[1 size(S,2) 1]),S); case {'helical' 'fourier'} - if strcmpi(param.mode, 'helical') - for ik=1:size(k, 1) - Sk = real(param.S(:, :, ik)); - if any(dot(repmat(n(ik, :), size(Sk, 2), 1)', Sk)) - warning('spinw:genmagstr:SnParallel', ... - ['There are spin components parallel to n, the ' ... - 'amplitude of these components will be modulated']); - break; - end - end - end S0 = param.S; % Magnetic ordering wavevector in the extended unit cell. kExt = k.*nExt; @@ -434,6 +443,17 @@ function genmagstr(obj, varargin) else S = S0; end + if strcmpi(param.mode, 'helical') + for ik=1:size(k, 1) + Sk = real(param.S(:, :, ik)); + if any(dot(repmat(n(ik, :), size(Sk, 2), 1)', Sk)) + warning('spinw:genmagstr:SnParallel', ... + ['There are spin components parallel to n, the ' ... + 'amplitude of these components will be modulated']); + break; + end + end + end case 'direct' % direct input of real magnetic moments S = param.S; @@ -503,9 +523,6 @@ function genmagstr(obj, varargin) if any(k) S = S + 1i*cross(repmat(permute(n,[2 3 1]),[1 size(S,2) 1]),S); end - - otherwise - error('spinw:genmagstr:WrongMode','Wrong param.mode value!'); end % normalize the magnetic moments From ca2c7afab150c6363ab00305729a0d90ad37c82b Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Wed, 17 Aug 2022 09:40:48 +0100 Subject: [PATCH 36/41] Raise error if nonsensical complex values are provided - Also move errors before warnings --- .../+unit_tests/unittest_spinw_genmagstr.m | 11 +++++ CHANGELOG.rst | 3 ++ swfiles/@spinw/genmagstr.m | 46 +++++++++++-------- 3 files changed, 41 insertions(+), 19 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 08cbf67e..fc8a2f36 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -49,6 +49,12 @@ {'mode', 'func', 'x0', [pi/2 -pi/4 0 0 [0 0 1/3] pi pi/2], 'unit', 'lu'}, ... {'mode', 'fourier', 'S',[1; i; 0], 'n', [1 0 0]} }; + complex_inputs = { + {'n', [1 i 0]}, ... + {'k', [i 0 0]}, ... + {'nExt', [1 1 i]}, ... + {'epsilon', complex(1)}, ... + {'phid', i}}; end methods (TestMethodSetup) function setup_chain_model(testCase) @@ -88,6 +94,11 @@ function test_no_magnetic_atoms_raises_error(testCase) @() swobj.genmagstr(), ... 'spinw:genmagstr:NoMagAtom') end + function test_complex_input_raises_error(testCase, complex_inputs) + testCase.verifyError(... + @() testCase.swobj.genmagstr('mode', 'helical', 'S', [1; 0; 0], complex_inputs{:}), ... + 'spinw:genmagstr:WrongInput') + end function test_tile_too_few_S_raises_error(testCase) swobj = copy(testCase.swobj); swobj.addatom('r', [0.5 0.5 0], 'S', 1); diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f05eef50..c5963a6b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -58,3 +58,6 @@ Bug Fixes ``helical`` and ``fourier`` modes in ``genmagstr`` - Emit warning if arguments that will be ignored are passed to a particular mode in ``genmagstr`` (e.g. ``S`` is passed to ``random``) +- Raise error if complex values are provided for ``k``, ``n``, ``nExt``, + ``epsilon`` or ``phid`` in ``genmagstr``. Previously + this might result in hanging or other errors depending on the argument. diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index 67e5daf5..5e91e5be 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -246,6 +246,33 @@ function genmagstr(obj, varargin) 'structure to be defined with another mode first']) end + +real_inputs = {"n", "k", "nExt", "epsilon", "phid"}; +for i = 1:length(real_inputs) + if any(strcmp(varargin, real_inputs{i})) && ~isreal(param.(real_inputs{i})) + error('spinw:genmagstr:WrongInput', '%s should be real', real_inputs{i}) + end +end + +if isempty(param.k) + noK = true; + param.k = k0; +else + noK = false; +end + +if prod(double(param.nExt)) == 0 + error('spinw:genmagstr:WrongInput','''nExt'' has to be larger than 0!'); +end + +% input type for S, check whether it is complex type +cmplxS = ~isreal(param.S); +if strcmpi(param.mode, 'helical') && cmplxS + error('spinw:genmagstr:WrongInput', ... + ['S must be real for helical mode. To specify complex basis ' ... + 'vectors directly use fourier mode.']) +end + if strcmp(param.mode,'extend') warning('spinw:genmagstr:DeprecationWarning',... 'extend mode is deprecated, please use tile mode instead'); @@ -272,25 +299,6 @@ function genmagstr(obj, varargin) end end -if isempty(param.k) - noK = true; - param.k = k0; -else - noK = false; -end - -if prod(double(param.nExt)) == 0 - error('spinw:genmagstr:WrongInput','''nExt'' has to be larger than 0!'); -end - -% input type for S, check whether it is complex type -cmplxS = ~isreal(param.S); -if strcmpi(param.mode, 'helical') && cmplxS - error('spinw:genmagstr:WrongInput', ... - ['S must be real for helical mode. To specify complex basis ' ... - 'vectors directly use fourier mode.']) -end - switch lower(param.unit) case 'lu' % convert the moments from lattice units to xyz From 65cfb42b2e4e828229c95301756e981afa125f88 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Wed, 17 Aug 2022 11:11:45 +0100 Subject: [PATCH 37/41] Add test with epsilon --- .../+unit_tests/unittest_spinw_genmagstr.m | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index fc8a2f36..cd178e77 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -134,6 +134,25 @@ function test_helical_spin_size_incomm_with_nExt_warns(testCase) 'F', [1 -1i; 1i 1; 0 0]); testCase.verify_obj(expected_mag_str, swobj.mag_str); end + function test_helical_spin_size_incomm_with_epsilon_warns(testCase) + swobj = copy(testCase.swobj); + delta = 1e-6; + k = [(delta+1)/3 0 0]; + nExt = [3 1 1]; + testCase.verifyWarning(... + @() swobj.genmagstr('mode', 'helical', ... + 'S', [1; 0; 0], ... + 'k', k, ... + 'nExt', nExt, ... + 'epsilon', 0.99*delta), ... + 'spinw:genmagstr:UCExtNonSuff') + expected_mag_str = struct('nExt', int32(nExt), ... + 'k', k', ... + 'F', [1 -0.5-0.866i -0.5+0.866i; ... + 1i 0.866-0.5i -0.866-0.5i; ... + 0 0 0]); + testCase.verify_obj(expected_mag_str, swobj.mag_str, 'rel_tol', 1e-4); + end function test_fourier_too_large_nExt_warns(testCase) swobj = copy(testCase.swobj); nExt = [6 1 1]; From aac29ed2433f205857241e2d8525bec9e2963bf6 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Wed, 17 Aug 2022 11:16:20 +0100 Subject: [PATCH 38/41] Put phi and phid into parametrised test --- +sw_tests/+unit_tests/unittest_spinw_genmagstr.m | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index cd178e77..f3545897 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -55,6 +55,7 @@ {'nExt', [1 1 i]}, ... {'epsilon', complex(1)}, ... {'phid', i}}; + rotate_phi_inputs = {{'phi', pi/4}, {'phid', 45}, {'phi', pi/4, 'phid', 90}}; end methods (TestMethodSetup) function setup_chain_model(testCase) @@ -545,22 +546,12 @@ function test_extend_mode_input_S_extend_cell_and_warns(testCase) expected_mag_str.F = [1 0 1 0 1 0; 0 1 0 1 0 1; 0 0 0 0 0 0]; testCase.verify_obj(expected_mag_str, swobj.mag_str); end - function test_rotate_phi(testCase) + function test_rotate_phi(testCase, rotate_phi_inputs) swobj = copy(testCase.swobj); k = [1/2 0 0]; % Need to initialise structure before rotating it swobj.genmagstr('mode', 'direct', 'S', [1; 0; 0], 'k', k); - swobj.genmagstr('mode', 'rotate', 'phi', pi/4); - expected_mag_str = testCase.default_mag_str; - expected_mag_str.F = sqrt(2)/2*[1; 1; 0]; - expected_mag_str.k = k'; - testCase.verify_obj(expected_mag_str, swobj.mag_str); - end - function test_rotate_phid(testCase) - swobj = copy(testCase.swobj); - k = [1/2 0 0]; - swobj.genmagstr('mode', 'direct', 'S', [1; 0; 0], 'k', k); - swobj.genmagstr('mode', 'rotate', 'phid', 45); + swobj.genmagstr('mode', 'rotate', rotate_phi_inputs{:}); expected_mag_str = testCase.default_mag_str; expected_mag_str.F = sqrt(2)/2*[1; 1; 0]; expected_mag_str.k = k'; From e8afe0103abfede118b06ff0f35c001e2f8d40aa Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Wed, 17 Aug 2022 11:44:54 +0100 Subject: [PATCH 39/41] Add test for norm explicitly true/false --- .../+unit_tests/unittest_spinw_genmagstr.m | 17 +++++++++++++++++ swfiles/@spinw/genmagstr.m | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index f3545897..8800e5bd 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -56,6 +56,11 @@ {'epsilon', complex(1)}, ... {'phid', i}}; rotate_phi_inputs = {{'phi', pi/4}, {'phid', 45}, {'phi', pi/4, 'phid', 90}}; + input_norm_output_F = { + % norm_bool, mag_str.F + {true, [2; 0; -2i]}, ... + {false, [1; 0; -1i]} + }; end methods (TestMethodSetup) function setup_chain_model(testCase) @@ -202,6 +207,18 @@ function test_helical_any_S_parallel_to_n_warns(testCase) 'F', [-sqrt(9/8)*1i; sqrt(9/8); sqrt(9/8)]); testCase.verify_obj(expected_mag_str, swobj_tri.mag_str); end + function test_helical_S2_norm(testCase, input_norm_output_F) + swobj = spinw(); + swobj.genlattice('lat_const', [3 8 8], 'angled', [90 90 90]); + swobj.addatom('r', [0 0 0], 'S', 2); + k = [1/3 0 0]; + swobj.genmagstr('mode', 'helical', 'S', [1; 0; 0], 'k', k, ... + 'n', [0 1 0], 'norm', input_norm_output_F{1}); + expected_mag_str = testCase.default_mag_str; + expected_mag_str.k = k'; + expected_mag_str.F = input_norm_output_F{2}; + testCase.verify_obj(expected_mag_str, swobj.mag_str); + end function test_direct_fm_chain(testCase) swobj = copy(testCase.swobj); swobj.genmagstr('mode', 'direct', 'k', [0 0 0], ... diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index 5e91e5be..ef613e9a 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -290,7 +290,7 @@ function genmagstr(obj, varargin) error('spinw:genmagstr:WrongMode','Wrong param.mode value!'); else input_arg_names = varargin(1:2:end); - input_arg_names(ismember(input_arg_names, "mode")) = []; + input_arg_names(ismember(input_arg_names, ["mode" "norm"])) = []; unused_args = setdiff(input_arg_names, mode_args.(param.mode)); if ~isempty(unused_args) warning('spinw:genmagstr:UnusedInput', ... From cf1060bc4ad7d619d4433ce403b0c94ce30dd600 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Wed, 17 Aug 2022 13:14:11 +0100 Subject: [PATCH 40/41] Only test for complex n Inspecting varargin is more complex than initially thought, sometimes it can be a struct (e.g. when called from optmagk) --- +sw_tests/+unit_tests/unittest_spinw_genmagstr.m | 11 +++-------- CHANGELOG.rst | 5 ++--- swfiles/@spinw/genmagstr.m | 9 +++------ 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 8800e5bd..4afdb141 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -49,12 +49,7 @@ {'mode', 'func', 'x0', [pi/2 -pi/4 0 0 [0 0 1/3] pi pi/2], 'unit', 'lu'}, ... {'mode', 'fourier', 'S',[1; i; 0], 'n', [1 0 0]} }; - complex_inputs = { - {'n', [1 i 0]}, ... - {'k', [i 0 0]}, ... - {'nExt', [1 1 i]}, ... - {'epsilon', complex(1)}, ... - {'phid', i}}; + complex_n_input = {[1 1+i 0], [i 0 0]}; rotate_phi_inputs = {{'phi', pi/4}, {'phid', 45}, {'phi', pi/4, 'phid', 90}}; input_norm_output_F = { % norm_bool, mag_str.F @@ -100,9 +95,9 @@ function test_no_magnetic_atoms_raises_error(testCase) @() swobj.genmagstr(), ... 'spinw:genmagstr:NoMagAtom') end - function test_complex_input_raises_error(testCase, complex_inputs) + function test_complex_n_input_raises_error(testCase, complex_n_input) testCase.verifyError(... - @() testCase.swobj.genmagstr('mode', 'helical', 'S', [1; 0; 0], complex_inputs{:}), ... + @() testCase.swobj.genmagstr('mode', 'helical', 'S', [1; 0; 0], 'n', complex_n_input), ... 'spinw:genmagstr:WrongInput') end function test_tile_too_few_S_raises_error(testCase) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fdf07d38..9fdae6f0 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -63,6 +63,5 @@ Bug Fixes ``helical`` and ``fourier`` modes in ``genmagstr`` - Emit warning if arguments that will be ignored are passed to a particular mode in ``genmagstr`` (e.g. ``S`` is passed to ``random``) -- Raise error if complex values are provided for ``k``, ``n``, ``nExt``, - ``epsilon`` or ``phid`` in ``genmagstr``. Previously - this might result in hanging or other errors depending on the argument. +- Raise error if complex values is provided for ``n`` in ``genmagstr``. + Previously this would've caused a crash. diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index a1937a2c..3760036d 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -246,12 +246,9 @@ function genmagstr(obj, varargin) 'structure to be defined with another mode first']) end - -real_inputs = {"n", "k", "nExt", "epsilon", "phid"}; -for i = 1:length(real_inputs) - if any(strcmp(varargin, real_inputs{i})) && ~isreal(param.(real_inputs{i})) - error('spinw:genmagstr:WrongInput', '%s should be real', real_inputs{i}) - end +% Complex n causes a crash, error out instead +if ~isreal(param.n) + error('spinw:genmagstr:WrongInput', 'n should be real') end if isempty(param.k) From f3c8aa2926df6c92b290487f213b8b4cfb0a0b53 Mon Sep 17 00:00:00 2001 From: Rebecca Fair Date: Wed, 17 Aug 2022 14:20:01 +0100 Subject: [PATCH 41/41] Account for varargin being a struct When called from places like optmagstr, varargin is actually a struct. Also, the struct contains many other additional parameters to do with fitting, so before calling genmagstr in optmagstr, warning('off','sw_readparam:UnreadInput') is called. This stops sw_readparam from issuing warnings. This also stops the `spinw:genmagstr:UnreadInput` warning from being triggered, which is why its name has been changed to UnreadInput, to avoid ugly unecessary warnings. --- .../+unit_tests/unittest_spinw_genmagstr.m | 8 ++++---- swfiles/@spinw/genmagstr.m | 20 ++++++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m index 4afdb141..7ee83075 100644 --- a/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m +++ b/+sw_tests/+unit_tests/unittest_spinw_genmagstr.m @@ -46,8 +46,8 @@ {'n', [0 1 1], 'mode', 'direct', 'S', [1; 0; 0]}, ... {'mode', 'tile', 'k', [0 0 1/2], 'n', [0 0 1], 'S', [1; 0; 0]}, ... {'mode', 'helical', 'k', [0 0 1/3], 'S', [1; 0; 0;], 'x0', []}, ... - {'mode', 'func', 'x0', [pi/2 -pi/4 0 0 [0 0 1/3] pi pi/2], 'unit', 'lu'}, ... - {'mode', 'fourier', 'S',[1; i; 0], 'n', [1 0 0]} + {'mode', 'func', 'x0', [pi/2 -pi/4 0 0 [0 0 1/3] pi pi/2], 'unit', 'lu', 'next', [1 2 1]}, ... + {'mode', 'fourier', 'S', [1; i; 0], 'n', [1 0 0]} }; complex_n_input = {[1 1+i 0], [i 0 0]}; rotate_phi_inputs = {{'phi', pi/4}, {'phid', 45}, {'phi', pi/4, 'phid', 90}}; @@ -110,7 +110,7 @@ function test_tile_too_few_S_raises_error(testCase) function test_ignored_input_warns(testCase, ignored_inputs) testCase.verifyWarning(... @() testCase.swobj.genmagstr(ignored_inputs{:}), ... - 'spinw:genmagstr:UnusedInput') + 'spinw:genmagstr:UnreadInput') end function test_rotate_ignored_input_warns(testCase) swobj = copy(testCase.swobj); @@ -118,7 +118,7 @@ function test_rotate_ignored_input_warns(testCase) swobj.genmagstr('mode', 'direct', 'S', [1; 0; 0]); testCase.verifyWarning(... @() swobj.genmagstr('mode', 'rotate', 'k', [0 0 1/3]), ... - 'spinw:genmagstr:UnusedInput') + 'spinw:genmagstr:UnreadInput') end function test_helical_spin_size_incomm_with_nExt_warns(testCase) swobj = copy(testCase.swobj); diff --git a/swfiles/@spinw/genmagstr.m b/swfiles/@spinw/genmagstr.m index 3760036d..02eec46c 100644 --- a/swfiles/@spinw/genmagstr.m +++ b/swfiles/@spinw/genmagstr.m @@ -230,10 +230,16 @@ function genmagstr(obj, varargin) % Error if S or k is provided but is empty. This means it has failed the % input validation, but hasn't caused an error because soft=True err_str = []; -if any(strcmp(varargin, 'S')) && isempty(param.S) +if isstruct(varargin{1}) + varargin_struct = varargin{1}; +else + varargin_struct = struct(varargin{:}); +end +varargin_names = fields(varargin_struct); +if any(strcmpi(varargin_names, 'S')) && isempty(param.S) err_str = ["S"]; end -if any(strcmp(varargin, 'k')) && isempty(param.k) +if any(strcmpi(varargin_names, 'k')) && isempty(param.k) err_str = [err_str "k"]; end if length(err_str) > 0 @@ -286,13 +292,13 @@ function genmagstr(obj, varargin) if ~any(strcmp(fields(mode_args), param.mode)) error('spinw:genmagstr:WrongMode','Wrong param.mode value!'); else - input_arg_names = varargin(1:2:end); - input_arg_names(ismember(input_arg_names, ["mode" "norm"])) = []; - unused_args = setdiff(input_arg_names, mode_args.(param.mode)); + unused_args = {varargin_names{:}}; + unused_args(ismember(lower(unused_args), ["mode" "norm"])) = []; + unused_args(ismember(lower(unused_args), lower(mode_args.(lower(param.mode))))) = []; if ~isempty(unused_args) - warning('spinw:genmagstr:UnusedInput', ... + warning('spinw:genmagstr:UnreadInput', ... 'Input(s) %s will be ignored in %s mode', ... - join(unused_args, ', '), param.mode) + string(join(unused_args', ', ')), param.mode) end end