Re: Fatal bug introduced for the conversion of kex to k_AB, using population pA. -- September 11, 2013 - 10:15

Hi,

There is also an alternative, and that would be to check if 'pA' and
'kex' are both present in spin[0].params.  But either solution would
be reasonable.

Regards,

Edward



On 11 September 2013 10:02, Edward d'Auvergne <edward@xxxxxxxxxxxxx> wrote:
> Hi,
>
> I had noticed that this commit had killed the entire branch ;)  That's
> ok though, because this is a branch and not the trunk.  The best fix
> would be to use the model information.  One of the arguments to this
> param_conversion() function is 'spins'.  This is a list of all the
> spins in the cluster.  The key is that every spin container holds the
> model name for the cluster.  There is another part of this module
> which uses the model information, just search for where the MODEL_M61B
> variable is used.  You should perform a similar test, and only perform
> the kex to k_AB conversion for the list of models in which this
> conversion is possible (i.e. they must have a pA and kex parameter).
>
> I have also noticed that the conversion is only half there.  There
> should also be the kex to k_BA conversion as well!
>
> Regards,
>
> Edward
>
>
>
>
> On 11 September 2013 09:47, Troels Emtekær Linnet <tlinnet@xxxxxxxxx> wrote:
> > Dear Edward.
> >
> > This commit introduced a fatal bug, making the whole systemtest fail.
> >
> > --------
> >         # The kex to k_AB conversion.
> >         if param_name == 'kex':
> >             # Get pA value.
> >             pA = get_value(key=key, spins=spins, sim_index=sim_index,
> > param_name='pA', spin_index=spin_index, frq_index=frq_index)
> >             k_AB = value * (1.0 - pA)
> >             set_value(value=k_AB, key=key, spins=spins,
> > sim_index=sim_index, param_name='k_AB', spin_index=spin_index,
> > frq_index=frq_index)
> > -----
> >
> > For example does the LM63 not contain the pA value.
> > And so it will fail with an AttributeError.
> >
> > A not very elegant solution is:
> > ------------
> >         # The kex to k_AB conversion.
> >         if param_name == 'kex':
> >             # Try tp get the pA value and set it.
> >             try:
> >                 pA = get_value(key=key, spins=spins,
> > sim_index=sim_index, param_name='pA', spin_index=spin_index,
> > frq_index=frq_index)
> >                 k_AB = value * (1.0 - pA)
> >                 set_value(value=k_AB, key=key, spins=spins,
> > sim_index=sim_index, param_name='k_AB', spin_index=spin_index,
> > frq_index=frq_index)
> >             except AttributeError:
> >                 pass
> > ------------
> > But "try" statements should always be avoided.
> >
> > Another possibility is to do a run through the generator: loop_parameters.
> > First add the available parameters to a list, and then loop over the 
> > generator.
> >
> > Then do a check if 'pA' is in the parameter list.
> >
> > What do you think?
> >
> > Best
> > Troels Emtekær Linnet
> >
> >
> >
> > ---------- Forwarded message ----------
> > From:  <edward@xxxxxxxxxxxxx>
> > Date: 2013/9/10
> > Subject: r20966 - in /branches/relax_disp: auto_analyses/
> > specific_analyses/relax_disp/ test_suite/system_tests/
> > To: relax-commits@xxxxxxx
> >
> >
> > Author: bugman
> > Date: Tue Sep 10 14:39:51 2013
> > New Revision: 20966
> >
> > URL: http://svn.gna.org/viewcvs/relax?rev=20966&view=rev
> > Log:
> > Added the conversion to k_AB from kex and pA. k_AB = kex * (1.0 - pA).
> >
> > Progress sr #3071: https://gna.org/support/index.php?3071 -
> > Implementation of Tollinger/Kay dispersion model (2001)
> > Following the guide at:
> > http://wiki.nmr-relax.com/Tutorial_for_adding_relaxation_dispersion_models_to_relax
> >
> > Troels E. Linnet provided this patch. Commit by: tlinnet _aaattt_ 
> > gmail_dot_com
> >
> > Signed-off-by: Edward d'Auvergne <edward@xxxxxxxxxxxxx>
> >
> > Modified:
> >     branches/relax_disp/auto_analyses/relax_disp.py
> >     branches/relax_disp/specific_analyses/relax_disp/api.py
> >     branches/relax_disp/specific_analyses/relax_disp/parameters.py
> >     branches/relax_disp/test_suite/system_tests/relax_disp.py
> >
> > Modified: branches/relax_disp/auto_analyses/relax_disp.py
> > URL: 
> > http://svn.gna.org/viewcvs/relax/branches/relax_disp/auto_analyses/relax_disp.py?rev=20966&r1=20965&r2=20966&view=diff
> > ==============================================================================
> > --- branches/relax_disp/auto_analyses/relax_disp.py (original)
> > +++ branches/relax_disp/auto_analyses/relax_disp.py Tue Sep 10 14:39:51 
> > 2013
> > @@ -417,10 +417,12 @@
> >              self.interpreter.value.write(param='dw', file='dw.out',
> > dir=path, force=True)
> >              self.interpreter.grace.write(x_data_type='res_num',
> > y_data_type='dw', file='dw.agr', dir=path, force=True)
> >
> > -        # The kex and tex parameters.
> > +        # The k_AB, kex and tex parameters.
> >          if model in [None, MODEL_LM63, MODEL_CR72, MODEL_CR72_FULL,
> > MODEL_IT99, MODEL_M61, MODEL_DPL94, MODEL_M61B,
> > MODEL_NS_CPMG_2SITE_3D, MODEL_NS_CPMG_2SITE_3D_FULL,
> > MODEL_NS_CPMG_2SITE_STAR, MODEL_NS_CPMG_2SITE_STAR_FULL,
> > MODEL_NS_CPMG_2SITE_EXPANDED, MODEL_NS_R1RHO_2SITE, MODEL_TP02]:
> > +            self.interpreter.value.write(param='k_AB',
> > file='k_AB.out', dir=path, force=True)
> >              self.interpreter.value.write(param='kex', file='kex.out',
> > dir=path, force=True)
> >              self.interpreter.value.write(param='tex', file='tex.out',
> > dir=path, force=True)
> > +            self.interpreter.grace.write(x_data_type='res_num',
> > y_data_type='k_AB', file='k_AB.agr', dir=path, force=True)
> >              self.interpreter.grace.write(x_data_type='res_num',
> > y_data_type='kex', file='kex.agr', dir=path, force=True)
> >              self.interpreter.grace.write(x_data_type='res_num',
> > y_data_type='tex', file='tex.agr', dir=path, force=True)
> >
> >
> > Modified: branches/relax_disp/specific_analyses/relax_disp/api.py
> > URL: 
> > http://svn.gna.org/viewcvs/relax/branches/relax_disp/specific_analyses/relax_disp/api.py?rev=20966&r1=20965&r2=20966&view=diff
> > ==============================================================================
> > --- branches/relax_disp/specific_analyses/relax_disp/api.py (original)
> > +++ branches/relax_disp/specific_analyses/relax_disp/api.py Tue Sep 10
> > 14:39:51 2013
> > @@ -1517,6 +1517,9 @@
> >          pairs['pA'] = 'pB'
> >          pairs['pB'] = 'pA'
> >
> > +        # Add the names of kex-k_AB pair.
> > +        pairs['k_AB'] = 'kex'
> > +
> >          # Get the minimisation statistic object names.
> >          min_names = self.data_names(set='min')
> >
> >
> > Modified: branches/relax_disp/specific_analyses/relax_disp/parameters.py
> > URL: 
> > http://svn.gna.org/viewcvs/relax/branches/relax_disp/specific_analyses/relax_disp/parameters.py?rev=20966&r1=20965&r2=20966&view=diff
> > ==============================================================================
> > --- branches/relax_disp/specific_analyses/relax_disp/parameters.py 
> > (original)
> > +++ branches/relax_disp/specific_analyses/relax_disp/parameters.py Tue
> > Sep 10 14:39:51 2013
> > @@ -716,6 +716,13 @@
> >              tex = 1.0 / (2.0 * value)
> >              set_value(value=tex, key=key, spins=spins,
> > sim_index=sim_index, param_name='tex', spin_index=spin_index,
> > frq_index=frq_index)
> >
> > +        # The kex to k_AB conversion.
> > +        if param_name == 'kex':
> > +            # Get pA value.
> > +            pA = get_value(key=key, spins=spins, sim_index=sim_index,
> > param_name='pA', spin_index=spin_index, frq_index=frq_index)
> > +            k_AB = value * (1.0 - pA)
> > +            set_value(value=k_AB, key=key, spins=spins,
> > sim_index=sim_index, param_name='k_AB', spin_index=spin_index,
> > frq_index=frq_index)
> > +
> >          # The tex to kex conversion.
> >          if param_name == 'tex':
> >              kex = 1.0 / (2.0 * value)
> >
> > Modified: branches/relax_disp/test_suite/system_tests/relax_disp.py
> > URL: 
> > http://svn.gna.org/viewcvs/relax/branches/relax_disp/test_suite/system_tests/relax_disp.py?rev=20966&r1=20965&r2=20966&view=diff
> > ==============================================================================
> > --- branches/relax_disp/test_suite/system_tests/relax_disp.py (original)
> > +++ branches/relax_disp/test_suite/system_tests/relax_disp.py Tue Sep
> > 10 14:39:51 2013
> > @@ -1014,6 +1014,9 @@
> >          self.assertAlmostEqual(res61L.kex, 609.262167216419, 0)
> >          self.assertAlmostEqual(res61L.chi2, 65.99987828889657, 5)
> >
> > +        # Test the conversion to k_AB from kex and pA.
> > +        self.assertEqual(res61L.k_AB, res61L.kex * (1.0 - res61L.pA))
> > +
> >
> >      def 
> > test_kteilum_fmpoulsen_makke_cpmg_data_048m_guhcl_to_cr72_full(self):
> >          """Optimisation of Kaare Teilum, Flemming M Poulsen, Mikael
> > Akke 2006 "acyl-CoA binding protein" CPMG data to the CR72 dispersion
> > model.
> >
> >
> > _______________________________________________
> > 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