Hi, The scaling factor is also a good idea. But please don't make the changes in the 'disp_speed' branch as you have started. This is unrelated, so a different branch must be used. I would like to merge the branches separately and not have the disp_speed branch wait for all analyses to be converted to your proposal, as it is close to ready.
Please don't get into a state where you are dumping everything into one branch, only for that branch to not be accepted for merger back into the trunk due to unclean and inconsistent APIs (the current target_functions.relax_disp to lib.dispersion API for example), partial changes to only the parts you are interested in, or other issues that will bring down the rest of relax. This has happened before - see the CST branch which dates back to the relax 1.2.10 release in January 2007 (http://svn.gna.org/viewcvs/relax/branches/). That code was no where near an acceptable state, and so it stays there as a branch, frozen in time. I am making these suggestions to make sure your useful improvements are kept flowing into relax, without it trashing the APIs in the source tree, for future developments and for the benefit of all. There will be no partial measures/implementations in the relax trunk. I will help you with suggestions as to how to avoid this and to have your code in a state acceptable for upstream merger. A new 'parameter_api' svn branch for your proposal is a good start (or some other name suggesting that you are cleaning the hard coded variables by shifting them into the parameter API objects). Discussing how to fix the mixed up target_functions.relax_disp to lib.dispersion API is another, and the final step to getting the 'disp_speed' branch ready for merger. Cheers, Edward On 28 May 2014 16:52, Troels Emtekær Linnet <[email protected]> wrote: > Hi Edward. > > It's fine for me! > > I think it is the very best, to have these values collected in one-table. > > Also for documentation! > These are values is what would go looking for, if I am the smallest > interested in what is happening. > > Also the linear constraints should be there. > And maybe also the scaling... > > Best > Troels > > > > > > 2014-05-28 16:36 GMT+02:00 Edward d'Auvergne <[email protected]>: >> That could be a default, using grid_lower and grid_upper for the >> mapping. But an override of map_lower and map_upper would be very >> useful, even in the dispersion analysis. This would be required for >> the kex parameter, as grid_lower is set to 1.0, whereas map_lower >> should be 0.0. The tex parameter as well. >> >> Regards, >> >> Edward >> >> >> On 28 May 2014 16:29, Troels Emtekær Linnet <[email protected]> wrote: >>> Hi Edward. >>> >>> I don't know much about the Model free. >>> >>> But I guess these values could be added to: >>> specific_values/XXX/parameter_object.py >>> >>> But as a start, the map values could be the grid values? >>> >>> Best >>> Troels >>> >>> >>> 2014-05-28 16:19 GMT+02:00 Edward d'Auvergne <[email protected]>: >>>> Hi, >>>> >>>> What did you think about the map bounds in the parameter_object >>>> specific analysis API? In addition to the grid_lower and grid_upper >>>> arguments, also have map_lower and map_upper arguments? If you look >>>> at specific_analyses/model_free/api.py and >>>> specific_analyses/model_free/optimisation.py, you will see that these >>>> are all set to different values for the model-free analysis. >>>> >>>> Regards, >>>> >>>> Edward >>>> >>>> >>>> >>>> >>>> On 28 May 2014 15:59, Edward d'Auvergne <[email protected]> wrote: >>>>> Hi, >>>>> >>>>> Exactly, but it would need to be in a branch (to keep the trunk >>>>> stable) and then done for all analyses (to keep the API clean). I.e. >>>>> the same steps as I mentioned before >>>>> (http://thread.gmane.org/gmane.science.nmr.relax.devel/5987). You've >>>>> already done half of the work with the changes at >>>>> http://article.gmane.org/gmane.science.nmr.relax.devel/5986. But it >>>>> must really be done for all analyses. The parameter_object is a major >>>>> part of the specific analysis API. Most of the work is in the base >>>>> parameter_object anyway, rather than in the specific analyses which >>>>> just require hardcoded values shifted from >>>>> specific_analyses/*/optimisation.py into >>>>> specific_analyses/*/parameter_object.py which is simply 39 pairs of >>>>> numbers: >>>>> >>>>> $ grep lower.append specific_analyses/*/* | wc -l >>>>> >>>>> Regards, >>>>> >>>>> Edward >>>>> >>>>> >>>>> >>>>> On 28 May 2014 15:47, Troels Emtekær Linnet <[email protected]> wrote: >>>>>> Hm. >>>>>> >>>>>> That is too much work. >>>>>> >>>>>> Can I write the default values into the >>>>>> specific_analysis/relax_disp/parameter_object.py. >>>>>> >>>>>> So I can simply call: >>>>>> >>>>>> from specific_analyses.relax_disp.parameter_object import >>>>>> Relax_disp_params >>>>>> PARAMS = Relax_disp_params() >>>>>> PARAMS.default_value('pA') >>>>>> PARAMS.grid_upper('pA') >>>>>> >>>>>> Then I wont touch the API. >>>>>> >>>>>> Best >>>>>> Troels >>>>>> >>>>>> 2014-05-28 15:30 GMT+02:00 Edward d'Auvergne <[email protected]>: >>>>>>> Hi, >>>>>>> >>>>>>> If you have the time and would like to implement this, such a change >>>>>>> must be done for the entire specific API. It cannot be just for the >>>>>>> relaxation dispersion analysis. The API change must be complete - >>>>>>> otherwise the changes will not be accepted as that will result in huge >>>>>>> amounts of maintenance work for me in the future, much more than the >>>>>>> effort of making the API consistent for all analyses. Though if you >>>>>>> set up one analysis type, the rest should be easy (half don't perform >>>>>>> minimisation, so the grid limits are not defined). The best way to >>>>>>> implement this would be: >>>>>>> >>>>>>> - Create a new svn branch from the trunk for the developments. >>>>>>> - Add the self._grid_lower and self._grid_upper objects and >>>>>>> corresponding arguments to specific_analyses.parameter_objects. >>>>>>> - Go into specific_analyses, and run 'grep lower.append */*' to see >>>>>>> all the places this needs changing. >>>>>>> - Shift all of these hard coded values into the specific >>>>>>> parameter_object modules. >>>>>>> - Import the parameter_object singleton into the modules requiring the >>>>>>> default bounds. >>>>>>> - Replace all the hard coded values with the >>>>>>> parameter_object.grid_lower() and parameter_object.grid_upper() >>>>>>> function calls. Note the places where the limits are dynamically >>>>>>> changed - these must remain. >>>>>>> - Make sure the test suite passes. >>>>>>> - Ask for the branch to be merged back. >>>>>>> >>>>>>> The API function grid_lower() should not be added, as these methods >>>>>>> are really designed for use outside of the specific API. If you wish >>>>>>> to do this, feel free. >>>>>>> >>>>>>> Regards, >>>>>>> >>>>>>> Edward >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> On 28 May 2014 15:00, Troels Emtekær Linnet <[email protected]> >>>>>>> wrote: >>>>>>>> Hi Ed. >>>>>>>> >>>>>>>> I would like to collect the grid seach lower and upper bounds into the >>>>>>>> table of >>>>>>>> specific_analysis/relax_disp/parameter_object.py >>>>>>>> >>>>>>>> This is to make one place, where such details are collected. >>>>>>>> >>>>>>>> And the lower and upper bounds can be extracted for unit tests, and >>>>>>>> for example dx.map more easily. >>>>>>>> >>>>>>>> I have worked it out for lower bounds of pA, and tested it. >>>>>>>> >>>>>>>> --- a/specific_analyses/api_base.py >>>>>>>> +++ b/specific_analyses/api_base.py >>>>>>>> @@ -314,6 +314,22 @@ class API_base(object): >>>>>>>> raise RelaxImplementError('grid_search') >>>>>>>> >>>>>>>> >>>>>>>> + def grid_lower(self, param): >>>>>>>> + """Return the default lower bounds of paramater for the grid >>>>>>>> search. >>>>>>>> + >>>>>>>> + This basic method will first search for a global parameter >>>>>>>> and, if not found, then a spin parameter. >>>>>>>> + >>>>>>>> + >>>>>>>> + @param param: The specific analysis parameter. >>>>>>>> + @type param: str >>>>>>>> + @return: The default value. >>>>>>>> + @rtype: float >>>>>>>> + """ >>>>>>>> + >>>>>>>> + # Return the value. >>>>>>>> + return self._PARAMS.grid_lower(param) >>>>>>>> + >>>>>>>> + >>>>>>>> def has_errors(self): >>>>>>>> """Test if errors exist for the current data pipe. >>>>>>>> >>>>>>>> diff --git a/specific_analyses/parameter_object.py >>>>>>>> b/specific_analyses/parameter_object.py >>>>>>>> index b626a83..8e7abab 100644 >>>>>>>> --- a/specific_analyses/parameter_object.py >>>>>>>> +++ b/specific_analyses/parameter_object.py >>>>>>>> @@ -54,6 +54,7 @@ class Param_list(object): >>>>>>>> self._scope = {} >>>>>>>> self._string = {} >>>>>>>> self._defaults = {} >>>>>>>> + self._grid_lowers = {} >>>>>>>> self._units = {} >>>>>>>> self._desc = {} >>>>>>>> self._py_types = {} >>>>>>>> @@ -92,7 +93,7 @@ class Param_list(object): >>>>>>>> return cls._instance >>>>>>>> >>>>>>>> >>>>>>>> - def _add(self, name, scope=None, string=None, default=None, >>>>>>>> units=None, desc=None, py_type=None, set='all', conv_factor=None, >>>>>>>> grace_string=None, err=False, sim=False): >>>>>>>> + def _add(self, name, scope=None, string=None, default=None, >>>>>>>> grid_lower=None, units=None, desc=None, py_type=None, set='all', >>>>>>>> conv_factor=None, grace_string=None, err=False, sim=False): >>>>>>>> """Add a parameter to the list. >>>>>>>> >>>>>>>> @param name: The name of the parameter. This will >>>>>>>> be used as the variable name. >>>>>>>> @@ -103,6 +104,8 @@ class Param_list(object): >>>>>>>> @type string: None or str >>>>>>>> @keyword default: The default value of the parameter. >>>>>>>> @type default: anything >>>>>>>> + @keyword grid_lower: The default lower bounds of the grid >>>>>>>> search. >>>>>>>> + @type grid_lower: float >>>>>>>> @keyword units: A string representing the parameters >>>>>>>> units. >>>>>>>> @type units: None or str >>>>>>>> @keyword desc: The text description of the parameter. >>>>>>>> @@ -134,6 +137,7 @@ class Param_list(object): >>>>>>>> self._names.append(name) >>>>>>>> self._scope[name] = scope >>>>>>>> self._defaults[name] = default >>>>>>>> + self._grid_lowers[name] = grid_lower >>>>>>>> self._units[name] = units >>>>>>>> self._desc[name] = desc >>>>>>>> self._py_types[name] = py_type >>>>>>>> @@ -540,6 +544,22 @@ class Param_list(object): >>>>>>>> return self._grace_string[name] >>>>>>>> >>>>>>>> >>>>>>>> + def grid_lower(self, name): >>>>>>>> + """Return the default lower bounds of paramater for the grid >>>>>>>> search. >>>>>>>> + >>>>>>>> + @param name: The name of the parameter. >>>>>>>> + @type name: str >>>>>>>> + @return: The default value. >>>>>>>> + @rtype: None or str >>>>>>>> + """ >>>>>>>> + >>>>>>>> + # Parameter check. >>>>>>>> + self.check_param(name) >>>>>>>> + >>>>>>>> + # Return the default value. >>>>>>>> + return self._grid_lowers[name] >>>>>>>> + >>>>>>>> + >>>>>>>> def is_spin_param(self, name): >>>>>>>> """Determine whether the given parameter is spin specific. >>>>>>>> >>>>>>>> diff --git a/specific_analyses/relax_disp/optimisation.py >>>>>>>> b/specific_analyses/relax_disp/optimisation.py >>>>>>>> index a92922e..82bd364 100644 >>>>>>>> --- a/specific_analyses/relax_disp/optimisation.py >>>>>>>> +++ b/specific_analyses/relax_disp/optimisation.py >>>>>>>> @@ -38,6 +38,7 @@ from lib.errors import RelaxError >>>>>>>> from lib.text.sectioning import subsection >>>>>>>> from multi import Memo, Result_command, Slave_command >>>>>>>> from pipe_control.mol_res_spin import spin_loop >>>>>>>> +from specific_analyses.api import return_api >>>>>>>> from specific_analyses.relax_disp.checks import check_disp_points, >>>>>>>> check_exp_type, check_exp_type_fixed_time >>>>>>>> from specific_analyses.relax_disp.data import average_intensity, >>>>>>>> count_spins, find_intensity_keys, has_exponential_exp_type, >>>>>>>> has_proton_mmq_cpmg, loop_exp, loop_exp_frq_offset_point, >>>>>>>> loop_exp_frq_offset_point_time, loop_frq, loop_offset, loop_time, >>>>>>>> pack_back_calc_r2eff, return_cpmg_frqs, return_offset_data, >>>>>>>> return_param_key_from_data, return_r1_data, return_r2eff_arrays, >>>>>>>> return_spin_lock_nu1 >>>>>>>> from specific_analyses.relax_disp.parameters import >>>>>>>> assemble_param_vector, assemble_scaling_matrix, >>>>>>>> disassemble_param_vector, linear_constraints, loop_parameters, >>>>>>>> param_conversion, param_num >>>>>>>> @@ -296,6 +297,9 @@ def grid_search_setup(spins=None, spin_ids=None, >>>>>>>> param_vector=None, lower=None, >>>>>>>> elif isinstance(inc, int): >>>>>>>> inc = [inc]*n >>>>>>>> >>>>>>>> + # The specific analysis API object. >>>>>>>> + api = return_api() >>>>>>>> + >>>>>>>> # Set up the default bounds. >>>>>>>> if not lower: >>>>>>>> # Init. >>>>>>>> @@ -357,7 +361,9 @@ def grid_search_setup(spins=None, spin_ids=None, >>>>>>>> param_vector=None, lower=None, >>>>>>>> if spins[si].model == MODEL_M61B: >>>>>>>> lower.append(0.85) >>>>>>>> else: >>>>>>>> - lower.append(0.5) >>>>>>>> + #lower.append(0.5) >>>>>>>> + lower.append(api.grid_lower('pA')) >>>>>>>> + #lower.append(api.default_value('pA')) >>>>>>>> upper.append(1.0) >>>>>>>> >>>>>>>> # The population of state B (for 3-site exchange). >>>>>>>> diff --git a/specific_analyses/relax_disp/parameter_object.py >>>>>>>> b/specific_analyses/relax_disp/parameter_object.py >>>>>>>> index 936d2b1..21dfe99 100644 >>>>>>>> --- a/specific_analyses/relax_disp/parameter_object.py >>>>>>>> +++ b/specific_analyses/relax_disp/parameter_object.py >>>>>>>> @@ -61,7 +61,7 @@ class Relax_disp_params(Param_list): >>>>>>>> self._add('r2', scope='spin', default=10.0, desc='The >>>>>>>> transversal relaxation rate', set='params', py_type=dict, >>>>>>>> grace_string='\\qR\\s2\\N\\Q (rad.s\\S-1\\N)', err=True, sim=True) >>>>>>>> self._add('r2a', scope='spin', default=10.0, desc='The >>>>>>>> transversal relaxation rate for state A in the absence of exchange', >>>>>>>> set='params', py_type=dict, grace_string='\\qR\\s2,A\\N\\Q >>>>>>>> (rad.s\\S-1\\N)', err=True, sim=True) >>>>>>>> self._add('r2b', scope='spin', default=10.0, desc='The >>>>>>>> transversal relaxation rate for state B in the absence of exchange', >>>>>>>> set='params', py_type=dict, grace_string='\\qR\\s2,B\\N\\Q >>>>>>>> (rad.s\\S-1\\N)', err=True, sim=True) >>>>>>>> - self._add('pA', scope='spin', default=0.90, desc='The >>>>>>>> population for state A', set='params', py_type=float, >>>>>>>> grace_string='\\qp\\sA\\N\\Q', err=True, sim=True) >>>>>>>> + self._add('pA', scope='spin', default=0.90, grid_lower=0.6, >>>>>>>> desc='The population for state A', set='params', py_type=float, >>>>>>>> grace_string='\\qp\\sA\\N\\Q', err=True, sim=True) >>>>>>>> self._add('pB', scope='spin', default=0.5, desc='The >>>>>>>> population for state B', set='params', py_type=float, >>>>>>>> grace_string='\\qp\\sB\\N\\Q', err=True, sim=True) >>>>>>>> self._add('pC', scope='spin', default=0.5, desc='The >>>>>>>> population for state C', set='params', py_type=float, >>>>>>>> grace_string='\\qp\\sC\\N\\Q', err=True, sim=True) >>>>>>>> self._add('phi_ex', scope='spin', default=5.0, desc='The >>>>>>>> phi_ex = pA.pB.dw**2 value (ppm^2)', set='params', py_type=float, >>>>>>>> grace_string='\\xF\\B\\sex\\N = \\q >>>>>>>> p\\sA\\N.p\\sB\\N.\\xDw\\B\\S2\\N\\Q (ppm\\S2\\N)', err=True, >>>>>>>> sim=True) >>>>>>>> >>>>>>>> Best >>>>>>>> Troels >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> relax (http://www.nmr-relax.com) >>>>>>>> >>>>>>>> This is the relax-devel mailing list >>>>>>>> [email protected] >>>>>>>> >>>>>>>> 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 _______________________________________________ relax (http://www.nmr-relax.com) This is the relax-devel mailing list [email protected] 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
