mailRe: r24336 - /branches/disp_spin_speed/target_functions/relax_disp.py


Others Months | Index by Date | Thread Index
>>   [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Header


Content

Posted by Troels Emtekær Linnet on July 22, 2014 - 17:54:
2014-06-27 17:28 GMT+02:00 Edward d'Auvergne <edward@xxxxxxxxxxxxx>:
Also the documentation in the get_back_calc() method of the target
function needs a little polish - it is not a float that is returned.
So, the running list:

- Some modules have "=" operators outside of functions that need
spaces around them.

Done

- Trailing whitespace, most of it is in the trunk, but there are some
new ones.  Just run:  grep " $" lib/dispersion/* | grep -v "\\\\ "

Done

- There are some empty lines straight after a 'for' loop in many places.

Done

- Your copyright in the tsmfk01.py file needs to be extended to 2014.

Done

- There are some FIXME comments in the lib.dispersion package.

They are not critical, and are in module which is not used yet.

- In some places, comment lines require empty lines before them (in
target_functions.relax_disp and lib.dispersion).

Where? Can you give example?

- Not very important, but the target function calc_ns_mmq_3site_chi2()
can have the "Once off parameter conversions" shifted into the
lib.dispersion module to simplify this.

Done

- The newline spacing between target function methods needs some fixes.

Done?

- The 'chi2_sum' initialisation in func_ns_mmq_2site() should go.

Done

- The get_back_calc() method of the target function needs a little
polish - it is not a float that is returned.

Done


Best
Troels

Cheers,

Edward



On 27 June 2014 17:26, Edward d'Auvergne <edward@xxxxxxxxxxxxx> wrote:
To add to the list of trivialities to fix:

- The 'chi2_sum' initialisation in func_ns_mmq_2site() should go.

Cheers,

Edward



On 27 June 2014 17:23, Edward d'Auvergne <edward@xxxxxxxxxxxxx> wrote:
Let me see.  You've covered most of the running list, there's just one
or two things left.  I'll add a few more trivial things as well.

- Some modules have "=" operators outside of functions that need
spaces around them.

- Trailing whitespace, most of it is in the trunk, but there are some
new ones.  Just run:  grep " $" lib/dispersion/* | grep -v "\\\\ "

- There are some empty lines straight after a 'for' loop in many places.

- Your copyright in the tsmfk01.py file needs to be extended to 2014.

- There are some FIXME comments in the lib.dispersion package.

- In some places, comment lines require empty lines before them (in
target_functions.relax_disp and lib.dispersion).

- Not very important, but the target function calc_ns_mmq_3site_chi2()
can have the "Once off parameter conversions" shifted into the
lib.dispersion module to simplify this.

- The newline spacing between target function methods needs some fixes.


I'll keep looking.  I'm guessing you will not be tackling the numeric
models to speed them up to match 'NS CPMG 2-site expanded' model via
the ultimate solution of brute-force expansion
(http://www.mail-archive.com/relax-users@xxxxxxx/msg01641.html) any
time soon.  This will have a similar effect as the ultimate speed up
at http://www.mail-archive.com/relax-devel%40gna.org/msg05691.html
which you have so successfully achieved in this branch.  Can you
remember if there were any other changes required?  I will create a
new timings file just before tagging the relax release.

Cheers,

Edward

On 27 June 2014 16:53, Troels Emtekær Linnet <tlinnet@xxxxxxxxxxxxx> 
wrote:
This sounds good.

What is needed to be done, to merge disp_spin_speed in trunk now?

Best
Troels

2014-06-27 15:53 GMT+02:00 Edward d'Auvergne <edward@xxxxxxxxxxxxx>:
Hmmm:

$ grep -c "out=\|einsum" lib/dispersion/*
lib/dispersion/b14.py:0
lib/dispersion/cr72.py:2
lib/dispersion/dpl94.py:0
lib/dispersion/__init__.py:0
lib/dispersion/it99.py:0
lib/dispersion/lm63_3site.py:0
lib/dispersion/lm63.py:0
lib/dispersion/m61b.py:0
lib/dispersion/m61.py:0
lib/dispersion/matrix_exponential.py:5
lib/dispersion/matrix_power.py:0
lib/dispersion/mmq_cr72.py:0
lib/dispersion/mp05.py:0
lib/dispersion/ns_cpmg_2site_3d.py:6
lib/dispersion/ns_cpmg_2site_expanded.py:0
lib/dispersion/ns_cpmg_2site_star.py:3
lib/dispersion/ns_matrices.py:0
lib/dispersion/ns_mmq_2site.py:12
lib/dispersion/ns_mmq_3site.py:12
lib/dispersion/ns_r1rho_2site.py:3
lib/dispersion/ns_r1rho_3site.py:3
lib/dispersion/tap03.py:0
lib/dispersion/tp02.py:0
lib/dispersion/tsmfk01.py:0
lib/dispersion/two_point.py:0


What do you think of the idea of making this a hard dependency:

"""
Index: dep_check.py
===================================================================
--- dep_check.py        (revision 24343)
+++ dep_check.py        (working copy)
@@ -37,6 +37,9 @@
 # numpy.
 try:
     import numpy
+    if float(numpy.version.version[:3]) < 1.6:
+        sys.stderr.write("Version %s of the 'numpy' dependency is not
supported, numpy >= 1.6 is required.\n" % numpy.version.version)
+        sys.exit()
 except ImportError:
     sys.stderr.write("The dependency 'numpy' has not been 
installed.\n")
     sys.exit()
"""

This might be the easiest way, just forcing users to upgrade.  I've
used the out argument a lot with the frame order analysis as well and
plan to use it more often, so maybe we just have to put our foot down
and make this a hard dependency.  That will make everyone's life
easier, and simplify what we as developers have to do to handle this.
I can apply this to the trunk and release relax 3.2.3 with all the
current fixes and this hard numpy version dependency, and then in a
few weeks when you are ready, merge the disp_spin_speed branch and
release relax 3.2.4.  What do you think?

Regards,

Edward





On 27 June 2014 14:40, Troels Emtekær Linnet <tlinnet@xxxxxxxxxxxxx> 
wrote:
Hi Ed.

You can just grep for "out" or "einsum" in lib/dispersion.

Best
Troels

2014-06-27 14:15 GMT+02:00 Edward d'Auvergne <edward@xxxxxxxxxxxxx>:
Hi Troels,

I think if would be better if we caught this earlier, specifically the
select_model() method of the specific_analyses.relax_disp.uf module if
the user chooses a model which is not supported by their numpy
version.  Such a check with a RelaxError would only be two lines of
code, plus a comment (which could include the current version in
numpy.version.version).  It would be very useful to add the models to
a list variable in specific_analyses.relax_disp.variables so that I
can replicate the checks in the GUI.  Even better would be to add the
2 line check to a function in specific_analyses.relax_disp.checks to
allow for consistent checking in all user interfaces.  There could
even be two separate tests, one for the numpy.einsum and one for the
numpy out argument, each with their own model lists, if you wish.

Cheers,

Edward






On 27 June 2014 12:58,  <tlinnet@xxxxxxxxxxxxx> wrote:
Author: tlinnet
Date: Fri Jun 27 12:58:22 2014
New Revision: 24336

URL: http://svn.gna.org/viewcvs/relax?rev=24336&view=rev
Log:
Added to target function that experiment_type_setup() should not be 
initiated, if numpy.einsum is missing.

Task #7807 (https://gna.org/task/index.php?7807): Speed-up of 
dispersion models for Clustered analysis.

Modified:
    branches/disp_spin_speed/target_functions/relax_disp.py

Modified: branches/disp_spin_speed/target_functions/relax_disp.py
URL: 
http://svn.gna.org/viewcvs/relax/branches/disp_spin_speed/target_functions/relax_disp.py?rev=24336&r1=24335&r2=24336&view=diff
==============================================================================
--- branches/disp_spin_speed/target_functions/relax_disp.py     
(original)
+++ branches/disp_spin_speed/target_functions/relax_disp.py     Fri 
Jun 27 12:58:22 2014
@@ -365,8 +365,10 @@
         # This is to make sure, that the chi2 values is not 
affected by missing values.
         self.mask_replace_blank = masked_equal(self.missing, 1.0)

-        # Check the experiment types, simplifying the data 
structures as needed.
-        self.experiment_type_setup()
+        # Check if eisum is available for numerical models.
+        if dep_check.einsum_module:
+            # Check the experiment types, simplifying the data 
structures as needed.
+            self.experiment_type_setup()

         # Scaling initialisation.
         self.scaling_flag = False


_______________________________________________
relax (http://www.nmr-relax.com)

This is the relax-commits mailing list
relax-commits@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-commits

_______________________________________________
relax (http://www.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



Related Messages


Powered by MHonArc, Updated Tue Jul 22 18:20:18 2014