Refactor spinwave.m into an object-oriented interface #14
Conversation
Factorise code for main q-point loop without twin, incommensurate, or biquadratic interactions into separate file spinwave_single.m Factorise code for matrix eigendecomposition into seprate function spinwave_hermit() Factorise code for spin-spin correlation tensor calculation into separate function spinwave_spinspincorrel() Factorise initialisation code into separate function. Factorise symbolic computation into separate function. Add calculation of twins using spinwave_single.m instead of concatenating q-point list.
Functional but incomplete; fails all tests
Magnetic fields not implemented
Twins not implemented
Incommensurate not implemented
Classes in new package +sw_classes
spin_wave_calculator - main class
magnetic_structure - helper class
- should be refactored to do what genmagstr and magstr
(amongst other routines) do now.
qvectors - helper class to handle hkl
- should be refactored along with a lattice class
…rect hamiltonian. test fails
Split calculations of Hamiltonian into two classes: sw_classes.hamiltonian sw_classes.biquadratic_hamiltonian These derive from sw_classes.hamiltonian_base which provides a function to calculate H(k) by applying eq (22) of Toth & Lake. The derive class constructor calculates the hkl-independent parts of the Hamiltonian according to eq (25) and (26).
Remove spinwave_single.m (initial functional implementation) Put back warnings and other informational info
| @@ -0,0 +1,50 @@ | |||
| classdef (Abstract) hamiltonian_base < handle | |||
| % An abstract base class for Hamiltonian calculations. | |||
| % It only defines the ham_k() method to calculate the hkl-dependent part of the Hamiltonian. | |||
There was a problem hiding this comment.
What is the the ham_k() method? Not clear why this sentence is essential to this abstract class
There was a problem hiding this comment.
So there are two parts to the Hamiltonian calculations:
- A q-vector (or k-vector or
hkl, sorry the notation is mixed up) independent part - A q-vector dependent part (also called the phase or exponential factor - it's the exp(ik.r) part)
The full Hamiltonian matrix we want is the product of these two parts. In order to save time, the calculation is set up to do the q-independent part first and then to save this in memory. For each set of q-vectors it is given it then calculates the phase/exponential factor and does the multiplication. This is the ham_k() method, and is the same for both the normal (bilinear) and the biquadratic Hamiltonian.
The bilinear and biquadratic Hamiltonian have different expressions for the q-vector independent parts and this is defined in their (concrete) constructors, whereas the q-vector dependent part (ham_k()) is defined in the abstract base class.
| properties(SetAccess=private) | ||
| % (Properties defined in sw_classes.hamiltonian_base) | ||
| % User input quantities: | ||
| %dR % The difference position vector between atom i and j |
There was a problem hiding this comment.
why are these properties here? They appear to be commented out
There was a problem hiding this comment.
They are defined in the abstract base class and are retained here for information only.
|
Also I am not a big fan of keeping old file |
# This is the 1st commit message: updates the descriptive comments for the biquadratic hamiltonia constructor to be consistent with updated argument names # This is the commit message #2: Update swfiles/+sw_classes/biquadratic_hamiltonian.m Use more descriptive names for the biquadratic hamiltonian constructor Co-authored-by: Anders Markvardsen <[email protected]> # This is the commit message #3: Renames the arguments of the hamiltonian class constructor to better reflect its meaning # This is the commit message #4: Wrote a brief description of the variable 'nChunks' # This is the commit message #5: adds a description of nHkl as a comment # This is the commit message #6: Update swfiles/+sw_classes/magnetic_structure.m Provides a brief comment on the 'magnetic_structure' class Co-authored-by: Duc Le <[email protected]>
constructor to be consistent with updated argument names. Update swfiles/+sw_classes/biquadratic_hamiltonian.m: Use more descriptive names for the biquadratic hamiltonian constructor. Co-authored-by: Anders Markvardsen <[email protected]> Renames the arguments of the hamiltonian class constructor to better reflect its meaning. Wrote a brief description of the variable 'nChunks'. Adds a description of nHkl as a comment. Update swfiles/+sw_classes/magnetic_structure.m: Provides a brief comment on the 'magnetic_structure' class Co-authored-by: Duc Le <[email protected]> Update swfiles/+sw_classes/qvectors.m: Adds an explanation of nHkl and hklIdx as comments. Co-authored-by: Duc Le <[email protected]> Adds an explanatory text describing the qvectors class
krishnakumarg1984
force-pushed
the
refactor_spinwave_m
branch
from
148a78a to
0d7b4c1
Compare
|
I've looked through the code superficially and everything looks reasonable. The only point I'd raise is that every public function should be documented in the correct markup syntax. I know it's long, dull and often repeating but these headers are used to generate documentation. So everything that is function return_value = function_name(obj, input1)
% This is a header and should summarise the function in 1 sentence.
%
% ### Syntax
% `return_value = function_name(obj, input1)`
%
% ### Description
% `function_name(obj, input1)` and a longer more detailed description. This might be identical to the short description if it is simplistic.
%
% ### Input Arguments
%
% `obj`
% : This is a definition of some object.
% Note the alignment of the multi-line text
%
% `input1`
% : Some other argument
%
% ### Output Arguments
%
% `return_value`
% : What is returned by the function
%
% ### See Also
%
% [obj] \| [obj.someOtherFn]Also the fields Also, I'd suggest some extra empty lines for readability. At the moment all the new code is in front of you and a line break when changing ideas really aids understanding. I.e; spectra.hkl = self.qvectors.hkl;
spectra.hklA = 2*pi*((self.spinWaveObject.unit.qmat*spectra.hkl)' / self.spinWaveObject.basisvector)';
spectra.helical = false; % TODO: sort out incommensurate and helical
spectra.norm = false;
spectra.nformula = double(self.spinWaveObject.unit.nformula);
% Save different intermediate results.
if self.parameters.saveV
if self.parameters.notwin
spectra.V = cat(3, Vsave{:});
else
spectra.V = Vsave_twin;
end
end
if self.parameters.saveH
if self.parameters.notwin
spectra.H = cat(3, Hsave{:});
else
spectra.H = cat(3, Hsave_twin{:});
end
endgoes to spectra.hkl = self.qvectors.hkl;
spectra.hklA = 2*pi*((self.spinWaveObject.unit.qmat*spectra.hkl)' / self.spinWaveObject.basisvector)';
spectra.helical = false; % TODO: sort out incommensurate and helical
spectra.norm = false;
spectra.nformula = double(self.spinWaveObject.unit.nformula);
% Save different intermediate results.
if self.parameters.saveV
if self.parameters.notwin
spectra.V = cat(3, Vsave{:});
else
spectra.V = Vsave_twin;
end
end
% Save different intermediate Hamiltonian.
if self.parameters.saveH
if self.parameters.notwin
spectra.H = cat(3, Hsave{:});
else
spectra.H = cat(3, Hsave_twin{:});
end
end |
Refactored
spinwave.minto an object-oriented template/interface.The methods are all called by the object created from spin wave class. Each method has a distinct purpose (such as prepare the Hamiltonian, calculate the Hamiltonian etc.). Different pathways have been accounted for (e.g. considering the bi-quadratics case, the incommensurate case etc.)
The original
spinwave.mfunction is retained for reference purposes, and has been renamed tospinwave_orig.mAll tests should pass (i.e. numerically give the same output as the expected output of the tutorials).
Fixes #4