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, self.diff_data) # The normal XH dipole interaction. self.diff_data.calc_di(data, self.diff_data) # Additional dipole, i.e. dipY. self.diff_data.calc_di(data, self.diff_data) # CSA part 1 (csa1). self.diff_data.calc_di(data, 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