Hi,
Please see below for my responses.
On 25 March 2010 07:56, Pavel Kaderavek <pavel.kaderavek@xxxxxxxxx> wrote:
Hi,
I looked for the possible simplification of the code construction of the in
the CST branch. Because you emphasize several times, that everything must be
discussed I would first suggest a general idea of the changes and then I
would start to make them step by step.
This is just so that we start with a good code design and not have to
rewrite a large amount of work. The problem is that the design might
be good by itself, but in the context of the rest of relax and what
people use relax for, it might actually be jumping on peoples toes.
Just to recall, Edward didn't like the fact, that I kept the code which was
already written and just add the functions dealing with asymmetric CST and
other features . That means that many similar mathematical functions were
abundantly repeated within the code.
This was just very bad coding style. If I would like to perform a
complex operation 100 times, I don't copy the code 100 times, I create
a loop over the 100 elements and call that one piece of code 100
times.
Lets start with the first step in the calculation procedure:
direction_cosine.py file
the "standard" version operates just with one dipole-dipole interaction to
attached proton and axially symmetric CST collinear with the XH bond . Our
idea was to add the possibility to calculate also with other nuclei in
proximity. Hence there would be necessary to change the scalar variable
defined by functions within the direction_cosine.py to vectors of the
dimensionality equal to all nuclei which dipole-dipole interaction should be
taken into the account. Additionally +2 if the asymmetric CS tensor should
be considered. Following the philosophy of this file the analogy of
xh_unit_vector for two eigenvectors of CST tensor should be calculated
outside this file.
Any comment? Do you agree?
A direction cosine is a direction cosine, independent of which
interaction we are looking at. Therefore direction_cosine.py should
be called multiple times for each of these interactions - well
whenever the interaction vector is different. Hence the file should
not change (much). Let's look at the calc_spheroid_di() function.
There are 2 containers passed into here, one general one for data, the
other containing the diffusion tensor. To use this for vector X from
interaction Y, you just need to pass the diffusion tensor structure in
and the data container. There are 3 places in mf.py where this
function is called, all in the func_*() methods at the lines:
self.diff_data.calc_di(data, self.diff_data)
I would suggest in this case that we change the functions in
direction_cosine.py to accept any unit vector. So in all places in
the file where 'data.xh_unit_vector' appears, this can be changed to
'data.unit_vector'. Then to access this many times, you have in the
equivalent place in func_*() in mf.py the following lines:
for i in range(len(data)):
self.diff_data.calc_di(data[i], self.diff_data)
The key here is that there are multiple containers of data for each
interaction. You can think of it like:
self.diff_data.calc_di(data[0], self.diff_data) # The normal XH
dipole interaction.
self.diff_data.calc_di(data[1], self.diff_data) # Additional
dipole, i.e. dipY.
self.diff_data.calc_di(data[2], self.diff_data) # CSA part 1 (csa1).
self.diff_data.calc_di(data[3], self.diff_data) # CSA part 2 (csa2).
If the self.data container in mf.py is properly designed, then that
will be the only file that needs changing in the entire 'maths_fns/'
directory. We can then loop over any physical interactions (even ones
you don't currently use) inside the func_*() methods, and that's it.
For a different physical interactions, the equations for direction
cosines inside the diffusion frame, diffusion tensor correlation
times, the model-free spectral density functions, Abragam's relaxation
equations, and the chi-squared function do not change. So the code
implementing these functions has no reason to change. The changes in
your original code
(http://svn.gna.org/viewcvs/relax/branches/cst_1.2.10/) to all files
excluding mf.py was just changing variable names for these different
interactions. The operation of the code is identical in each case.
Because the code operation does not change (and neither do the
physical equationsl), then code duplication N times to perform these
identical operations N times is not acceptable. The solution is to
create a loop over the N elements and call the math_fns modules once.
Regards,
Edward
Pavel
_______________________________________________
relax (http://nmr-relax.com)
This is the relax-devel mailing list
relax-devel@xxxxxxx
To unsubscribe from this list, get a password
reminder, or change your subscription options,
visit the list information page at
https://mail.gna.org/listinfo/relax-devel
Re: CST branch