Hi Ed, You're absolutely right... I haven't thought about machine precision and must have been tired to see the test as more rigorous like I proposed... Sorry for that. Have a nice weekend ! Séb Edward d'Auvergne wrote:
Hi, The tests were originally of the form 'abs(calculated - true) == 0'. But because of slightly different numerical values depending on machine architecture, as well as machine precision issues, the value of zero was changed to a small constant. The division of the true value by 1e6 to create this constant means that the calculated value must be within one one millionth of the true value. If you remove the '/1e6' then the test will pass if the calculated value is within plus or minus the true value. Hence the test will have been relaxed and will almost always pass, but it will not be a very vigorous test. Cheers, Edward On 7/16/07, Sebastien Morin <sebastien.morin.1@xxxxxxxxx> wrote:Hi, When I adapted the jw_mapping code to yield the consistency_test code, I didn't understand why, in the test suite, the following lines : if abs(self.relax.data.res[self.run][index].j0 - j0[index]) > j0[index]/1e6: print 'Error in residue', self.relax.data.res[self.run][index].num, 'j0 calculated value' return if abs(self.relax.data.res[self.run][index].f_eta - f_eta[index]) > f_eta[index]/1e6: print 'Error in residue', self.relax.data.res[self.run][index].num, 'f_eta calculated value' return if abs(self.relax.data.res[self.run][index].f_r2 - f_r2[index]) > f_r2[index]/1e6: print 'Error in residue', self.relax.data.res[self.run][index].num, 'f_r2 calculated value' return had a '/1e6' in the end and why those lines were not simply : if abs(self.relax.data.res[self.run][index].j0 - j0[index]) > j0[index]: print 'Error in residue', self.relax.data.res[self.run][index].num, 'j0 calculated value' return if abs(self.relax.data.res[self.run][index].f_eta - f_eta[index]) > f_eta[index]: print 'Error in residue', self.relax.data.res[self.run][index].num, 'f_eta calculated value' return if abs(self.relax.data.res[self.run][index].f_r2 - f_r2[index]) > f_r2[index]: print 'Error in residue', self.relax.data.res[self.run][index].num, 'f_r2 calculated value' return I tried to modify those lines and the test-suite code still works, anyway. Could you explain this issue to me, please ? Or is it a mistake and should the '/1e6' part be taken off..? Thanks ! Séb :) Edward d'Auvergne wrote:Oh well, it doesn't matter if it's not documented in the code then. Regards, Edward On 7/13/07, Sebastien Morin <sebastien.morin.1@xxxxxxxxx> wrote:Hi, For the eta value calculated in the consistency_tests code, I don't know if it is the eta_z or eta_xy value. In the original paper by Fushman & Cowburn (1998, JACS, 120: 7109-7110) as well as in the consistency testspaper(Fushman et al., 1998, JACS, 120: 10947-10952), eta is only described as the cross-correlation rate between 15N CSA and 15N-1H dipolarinteraction.Cheers Séb :) Sébastien Morin wrote: Hi Ed, Here is the first part of the split patch as you asked for. This one if for docstrings and comments in files : /branches/consistency_tests_1.2/prompt/consistency_tests.py /branches/consistency_tests_1.2/maths_fns/consistency_tests.py /branches/consistency_tests_1.2/test_suite/consistency_tests.py /branches/consistency_tests_1.2/sample_scripts/consistency_tests.py /branches/consistency_tests_1.2/specific_fns/consistency_tests.py Should the commit log be something like what follows ? "This patch (patch_consistency_tests__l1.2_r3340__docstrings_and_comments) corrects docstrings and comments errors which were remnants of the jw_mapping code from which the consistency_tests code was inspired. Also, this patch adds docstrings and comments to the code for making it easier for users to understand what the code actually does." Cheers Séb :) Selon Edward d'Auvergne <edward.dauvergne@xxxxxxxxx>, 13.07.2007: Hi, This patch will need a few small changes before being committed. The first is the docstring of the set_frq function of 'prompt/consistency_tests.py'. In the user function docstrings, the list of keyword arguments need to be separated by newlines. This is a relax convention as can be seen in the equivalent function of 'prompt/jw_mapping.py'. One problem with this is that the docstring parser used to generate the relax manual might fail in this keyword argument section. An additional formatting convention is the use of two blank lines in front of the section titles in the docstring. This allows easier reading of the much longer docstrings. This isn't important but for the eta value, do you know if this is eta_z or eta_xy? A more important point is that the patch should be split up so that there are different patches for different types of fixes. For example there are docstring and comment changes which could be grouped together into a single patch. Then there are modifications to the test suite for the consistency tests whichshouldbe separate. In 'specific_fns/consistency_tests.py' there is a change to the default value of the CSA and a number of other fundamental changes. These should all be separate. It would be a good exercise in preparation for having full commit access to the relax repository to split this patch into a number of small patches. No patch or commit to the relax repository should contain two unrelated changes, even if these are tiny one line changes. For each commit to the repository, a detailed description of the changes should be placed into the commit log (which is automatically emailed to the relax-commits mailing list butremains inthe repository and is important for repository maintenance). You can see these messages by typing something like 'svn log -v --limit=100 | less' within your checked out copy. So if you could prepare a similar commit message for the patches, it would be much appreciated. Cheers, Edward P.S. A small fix is needed in 'prompt/consistency_tests.py' at line 4. And as for the self.__relax_help_ string, this isn't actually a docstring but is used by the relax prompt help system. These __init__() functions don't have docstrings, and I don't remember if this was deliberate because of the special help system or not. On 7/13/07, Sébastien Morin <sebastien.morin.1@xxxxxxxxx> wrote: Hi, Here is a patch for the consistency tests branch of the 1.2 line (r3340). Some comments and docstrings were added as well as errorscorrected fromthe adaptation of this code from the jw_mapping code... The following file are modified : prompt/consistency_tests.py maths_fns/consistency_tests.py test_suite/consistency_tests.py sample_scripts/consistency_tests.py specific_fns/consistency_tests.py Formating should be OK in these files except maybe for one thing for which I'm not sure. It's between lines 29 and 33 of prompt/consistency_tests.py where I don't know if the docstring should be below the line 'def __init__(self, relax):' or below the line 'self.__relax_help__ = \' as it is now... Note that this is seen in many other code files... Cheers Sébastien :) Selon Edward d'Auvergne <edward.dauvergne@xxxxxxxxx>, 09.07.2007: Hi, Both patches have been applied to the repository. The patch 'patch_sample_scripts_consistency_tests' was committed at r3331. Change from 5000 to 500 simulations is reasonable. The large number of simulations was because the calculation was so quick, but 500 should be more than sufficient. The addition of Grace plotting functions to the Reduced Spectral Density Mapping sample script is very useful and I will probably port this commit very soon to the 1.3 line. The patch 'patch_sample_scripts_jw_mapping' was applied (at r3332) directly to the 1.2 line rather than the 'consistency_tests_1.2' branch. Cheers, Edward On 7/9/07, Sébastien Morin <sebastien.morin.1@xxxxxxxxx> wrote: Hi again ! I added lines for plotting using grace in the sample script. Sincethissample script was inspired by the jw_mapping sample script, I also submit a patch for this file (adding lines for plotting, changing the number of Monte Carlo simulations from 5000 to 500 and changing the sequence file from noe.500.out to noe.600.out for more consistency)... Ciao ! Sébastien :) Selon Sébastien Morin <sebastien.morin.1@xxxxxxxxx>, 09.07.2007: Hi Ed Tell me if this works better. I'm now using the mail server directly from the internet GUI... (not with Thuderbird). If this does not work neither, maybe we could try with the task... Ciao ! Sébastien Selon Edward d'Auvergne <edward.dauvergne@xxxxxxxxx>, 09.07.2007: I've tried cutting an pasting the patch but I get the following: edau@klymene:/media/usbdisk/relax/branches/consistency_tests_1.2> patch -p0 < patch patching file prompt/consistency_tests.py Hunk #2 FAILED at 26. Hunk #3 FAILED at 40. Hunk #4 FAILED at 49. Hunk #5 FAILED at 74. 4 out of 5 hunks FAILED -- saving rejects to file prompt/consistency_tests.py.rej edau@klymene:/media/usbdisk/relax/branches/consistency_tests_1.2> I don't know why, but thunderbird is destroying the attachment. The failed 'hunks' are the wrapped lines. Maybe there is a way to prevent thunderbird from doing this. Otherwise using another email client (or webmail) may work. I could also create a task for this consistency test work and these could be attached to the task. It's best that, for the record, the files are located within the permanent relax infrastructure. Cheers, Edward On 7/9/07, Sebastien Morin <sebastien.morin.1@xxxxxxxxx> wrote: Hi again ! Here is again the first patch for the file 'prompt/consistency_tests.py'. I modified the header for the copyrights. Also, the former patches were not copied-pasted, but attached using Thunderbird after their creation using a command like 'svn diff > patch' under Linux. When I send those kinds of files as attachment, I usually see them as text in the e-mail, but also as an attachment that can be save. Tell me if it is okay and if it is still a problem, I'll put them on my lab's website... Cheers Séb :) Edward d'Auvergne wrote: Hi, Just add some text such as 'Copyright (C) 2007 Sebastien Morin <sebastien.morin.1 at ulaval.ca>' underneath the already existent copyright text. The can be changed later, for example I can give you a ???@nmr-relax.com email address which is an alias for any other email address (once voted in as a relax developer). Could you add this and then resend the patches? Ta. If they are attached rather than cut and paste that would be much easier for applying the patches (as email wraps lines). Also, maybe responding to your original posts will allow the patches to be more easily tracked in the mailing list https://mail.gna.org/public/relax-devel/. Cheers, Edward On 7/9/07, Sebastien Morin <sebastien.morin.1@xxxxxxxxx> wrote: Hi ! I'm about to create the patches to merge the consistency tests code into the 1.2 branch. However, I have one question. How do I treat the copyrights ? Do I leave the original author from which I copied the code and then modified it or do I had my name to the code headers..? Thanks ! Sébastien :) Edward d'Auvergne wrote: Oh, I've committed your patch athttp://maple.rsvs.ulaval.ca/mediawiki/index.php/Patch_consistency_tests_2007-06-26as revision r3324, applying it to the 1.2 branch. I've carefully checked the code and none of the changes are detrimental or could affect the stability of the stable 1.2 relax codebase. Note however that the code in the branch will not run as the consistency_test.py files are still identical copies of the jw_mapping.py files. Cheers, Edward On 7/9/07, Edward d'Auvergne <edward.dauvergne@xxxxxxxxx> wrote: Hi, I've now created two branches within the relax repository for you to play with. The first is a copy of the 1.2 line and is located at svn.gna.org/svn/relax/branches/consistency_tests_1.2/. The second is a copy of the 1.3 line and is located at svn.gna.org/svn/relax/branches/consistency_tests_1.3/. I've initially used 'svn cp' to create the 5 consistency_tests.py files as described in https://mail.gna.org/public/relax-devel/2007-07/msg00001.html (Message-id: <7f080ed10707090251ve1c4a8fl7f8618843e5c9459@xxxxxxxxxxxxxx>). Would you be able to create patches for these files (in the 1.2 line first, no need to worry about the 1.3 line yet), and then post the individual patches as text file attachments to the mailing list? Thanks. I will then be able to commit these patches individually, checking them in fine detail. Things to note in creating the patches from the code at http://maple.rsvs.ulaval.ca/mediawiki/index.php/Relax_development include the copyright preservation, a number of integers in 'maths_fns/consistency_tests.py' which should be floating point numbers (just add '.0' to the end of the number), the addition of Grace plots as an output in the 'sample_scripts/consistency_tests.py' script to be able to create a picture similar to that on your relax development site, maybe only allowing 'Tc' and 'tc' in the return_data_name() function and not 'TC', and 'degrees' instead of 'degree' in the return_units() function. One bug includes: setattr(self.relax.data.res[self.run][index], 'csa', float(value[0])) setattr(self.relax.data.res[self.run][index], 'r', float(value[1])) + setattr(self.relax.data.res[self.run][index], 'orientation', float(value[1])) + setattr(self.relax.data.res[self.run][index], 'tc', float(value[1])) value[1] has been used twice. I have a feeling there is another bug somewhere where an index has been repeated a few times when it should be different indices, but I can't find it at the moment. The individual patches should help. Finally, I have a feeling that there is unused code which can be deleted as it is a relic from the copy of the J(w) mapping code and is not needed. For the 1.3 line code I would recommend that identical functions are shifted into files such as 'specific_fns/base_class.py', but for the 1.2 line code I would prefer the duplication as this means that the current stable code base remains stable. Cheers, Edward On 6/26/07, Sebastien Morin <sebastien.morin.1@xxxxxxxxx> wrote: Hi, I started working on implementing the consistency tests last week before the last post was made and, hence, I worked on repository line 1.2 (revision 3303). I implemented the consistency tests as a new type of run ('ct') similar to the one for Jw mapping. The calculations are made for J(0), F_eta and F_R2 separately for each magnetic field (one at a time). The output results file is similar to the one for Jw mapping. The user then needs to plot them and look for consistency with its own criteria (calculation correlation coefficients, mean ratios and standard deviations, etc). Please look at the followinr URL for a listing of the modifications to old files and also necessary new files. http://maple.rsvs.ulaval.ca/mediawiki/index.php/Relax_development The file 'sample_scripts/consistency_tests.py' should be useful to understand how the new procedure works. Even if this was done on repository line 1.2, I think it is quite fine since nothing was deleted but only things added (maybe too much, as I reproduced the Jw mapping approach, maybe too much as I added lines in the codes for grace, molmol, etc, maybe too much also since some code is duplicated from the Jw mapping code). The test-suite still works perfectly and, so, I think it could be fine to add the tests to the 1.2 line as well... However, if necessary, I could implement the consistency testing procedure on line 1.3, following your comments as I am quite new to Python and maybe made things somehow not perfectly... Please tell me what you think about this. Cheers, Sébastien :) Edward d'Auvergne wrote: Hi, I have previously talked about data set consistency. For example see the post at https://mail.gna.org/public/relax-users/2007-06/msg00001.html in which a few reasons for inconsistencies have been explained. I have, from experience, noticed that small changes in protein concentration can change the collected relaxation rates significantly - most likely because of packing interactions. All samples should essentially be identical in all respects for the relaxation rates to be compared. And the temperate should always be fine tuned between experiments and spectrometers using methanol (and always checked later on if there is a large time between collecting the same experiment). Therefore these tests would be quite useful. Data consistency is essential for the model-free results to be correct (as well as reduced spectral density mapping, SRLS, etc.) as this affects both the optimisation and model selection and can result in artificial motions appearing. However I don't know how these test would currently fit within relax. Maybe a new type of analysis should be created for this (see the pipe.create() user function in the 1.3 line or the run.create() user function in the 1.2 line). These ideas should all go into the 1.3 line (via a branch) as the 1.2 line is stable and no new major features will be added to this code. What are the ideas you have been playing with? Cheers, Edward On 6/15/07, Sebastien Morin <sebastien.morin.1@xxxxxxxxx> wrote: Hi everyone During the last months, I was astonished to realize that some spin relaxation data I had acquired at different fields were not consistent between each other. The way I realized that was by seeing discrepancy between J(0) values calculated with those different datasets. I looked a little bit in the litterature and found some interesting consistency tests in a paper by Fushman (Fushman et al., JACS, 1998, 120:10947-10952). This paper present 2 consistency tests to compare datasets from different magnetic fields / samples / time / etc. I think it would be interesting to implement those simple tests in relax so the user can, before trying to fit their data, know the quality of those... Regrettably, very few people look at the consistency of their datasets before analysis... The underlying principle is the same as when looking at consistency for J(0). Thus, I think that those two tests and a J(0) test should be implemented altogether... I'll try to work a bit on this. Mimicking the code for ... [Message clipped]-- ______________________________________ _______________________________________________ | | || Sebastien Morin || ||| Etudiant au PhD en biochimie ||| |||| Laboratoire de resonance magnetique nucleaire |||| ||||| Dr Stephane Gagne ||||| |||| CREFSIP (Universite Laval, Quebec, CANADA) |||| ||| 1-418-656-2131 #4530 ||| || || |_______________________________________________| ______________________________________
-- ______________________________________ _______________________________________________ | | || Sebastien Morin || ||| Etudiant au PhD en biochimie ||| |||| Laboratoire de resonance magnetique nucleaire |||| ||||| Dr Stephane Gagne ||||| |||| CREFSIP (Universite Laval, Quebec, CANADA) |||| ||| 1-418-656-2131 #4530 ||| || || |_______________________________________________| ______________________________________