Camelia, the Perl 6 bug

IRC log for #cdk, 2010-08-13

| Channels | #cdk index | Today | | Search | Google Search | Plain-Text | summary

All times shown according to UTC.

Time Nick Message
00:16 slyrus_ joined #cdk
00:16 slyrus_ any SMILES experts around?
02:04 s9asad joined #cdk
03:57 azeem_ joined #cdk
04:44 egonw joined #cdk
05:23 sneumann joined #cdk
05:55 jbrefort joined #cdk
06:04 bag_ joined #cdk
06:35 egonw joined #cdk
07:09 Gpox joined #cdk
07:37 jbrefort joined #cdk
07:45 jonalv joined #cdk
12:28 egonw anyone who feels for reviewing a few small patches? some #cdk experience required
12:39 * jonalv is not in procrastination mode
13:10 maclean joined #cdk
13:10 maclean hi egonw
13:11 egonw hi maclean
13:11 maclean been testing the CIP patch.
13:12 egonw oh cool!
13:12 maclean https://gist.github.com/f0861b94023e59370c16
13:12 egonw thanx
13:12 maclean http://www.daylight.com/daycgi​/depict?4f4331434331284629436c
13:12 zarah maclean's link is also http://tinyurl.com/24x6gr3
13:12 maclean Stack overflow errors, though :(
13:12 egonw still?
13:12 egonw :(
13:13 maclean smiles : OC1CC1(F)Cl
13:13 egonw ack
13:13 egonw will add that as unit test
13:13 maclean Ok.
13:13 egonw thanx!
13:15 egonw though I had rather seen the problem did not exist, of course
13:18 maclean In fact, the series : OC1C{1,..}C1(F)Cl (to abuse the smiles notation) seems to fail, for at least 3-7 rings
13:19 maclean So it's not due to the 3-ring being so small that it causes problems.
13:19 egonw yeah, whereas the first ligands-of-ligands should already differentiate... because it has the oxygen... :(
13:20 maclean Really, a better test would have the distinguishing oxygen further away, to see if that works.
13:20 egonw :)
13:21 egonw ah, maclean is starting to like writing unit tests... good :)
13:21 maclean like : C1C(O)CC1(F)Cl
13:21 maclean These are really functional tests, though, aren't they?
13:21 egonw strictly speaking, I guess so...
13:21 egonw then again...
13:21 egonw the probably test a particular line in the code...
13:22 egonw one particular unit of functionality, that is...
13:22 maclean True. I am writing more tests.
13:22 egonw not sure about the exact rules and definitions...
13:22 egonw but the CDK unit tests are indeed sometimes quite of a functional nature
13:22 egonw but as Emma shows, that's not uncommon
13:22 maclean Emma?
13:22 egonw the code coverage tool
13:23 maclean http://emma.sourceforge.net/
13:23 egonw tests what code is called when running JUnit tests
13:23 maclean ah, right.
13:23 egonw yes
13:24 egonw maclean: can you please follow up on: https://sourceforge.net/tracker/?func=detail&amp​;aid=2998556&group_id=20024&atid=120024
13:25 maclean well, it hasn't happened since, so I guess it can b considered closed. Maybe my cdk just got screwed up locally when running ant.
13:27 egonw ok, please close the bug
13:27 maclean Ok.
13:29 maclean "works in current SVN version" should really be "works in current git version", I suppose. Or "works in current repository version"...
13:38 egonw :)
13:38 egonw let me see if I can fix that
13:39 maclean Hmmm. It doesn't seem to matter how the oxygen is offset in the ring. eg : OC1CCCC(F)(CC1)Cl
13:39 maclean (gist updated)
13:39 egonw done
13:39 maclean cool
13:40 maclean It really seems like the recursive compare method has no termination conditions.
13:42 egonw no, it does not have a proper one...
13:42 egonw the fix should actually terminate this one too...
13:42 maclean But it seems to have lots, like "if ligand1.length == ligand2.length return 1"
13:43 egonw yeah, that's an easy condition...
13:43 egonw if an atom has more ligands, then it is clear which takes priority...
13:43 egonw otherwise it needs to go into recursion...
13:43 egonw almost done with bug tracker maintainance...
13:43 egonw maintenance?
13:44 egonw I wish my IRC client had spell checking :)
13:45 egonw maclean: did you write the SVG JCP code?
13:46 egonw if so, could you have a look at this bug report:
13:46 maclean yeah, the start of it.
13:46 egonw https://sourceforge.net/tracker/?func=detail&amp​;aid=3010502&group_id=20024&atid=120024
13:46 maclean the triple bond thing?
13:46 egonw yeah
13:46 maclean yeah, the triple bond thing.
13:47 egonw if you cannot fix it (right now), that's fine too... but please leave a comment so that the rest of us knows how and what :)
13:47 maclean will do.
13:47 egonw OK, got called away for tea and snacks
13:47 egonw bbl
13:48 maclean snacks++
13:48 egonw maclean: oh, and if you want your name to show up reviewer list, there is a list of small patches up for review
13:49 egonw begging for: git commit --amend --signoff
13:49 egonw :)
13:49 maclean GO! SNAX!
14:23 CIA-50 cdk: Egon Willighagen master * r9364179 / (javadoc.xml develjar/ojdcheck-jazzy.jar): Removed the OpenJavaDocCheck Jazzy extension which was not supposed to go in; it's not ready for prime time - http://bit.ly/aCkdVN
14:23 maclean joined #cdk
14:33 maclean hahah! /. story on 'Rupert Murdoch Claims To Own the "Sky" In "Skype"' :D
14:34 maclean Presumably makers of skylights are worried...
14:35 egonw ah wel...
14:35 egonw I think I own my genes, but that has never stopped anyone patenting them
14:37 maclean The best thing is that it almost sounds like he is claiming ownership of the sky itself. Presumably from his floating sky castle...
14:38 maclean Anyway egonw, what about this stack overflow error in the CIP tool - do you think it would be helpful to discuss it now? Or would you prefer to diagnose and bugfix yourself? Or?
14:39 egonw yeah, just about to encode your SMILES as unit test...
14:39 egonw let me have a quick look
14:39 maclean ok
14:39 egonw perhaps you can review and sign off those two small javadoc fixes?
14:39 maclean I'll sketch some things out on paper.
14:42 egonw maclean: it works here...
14:42 egonw no time out
14:42 egonw terminates fine...
14:42 maclean oh!
14:42 maclean hmmm.
14:42 maclean last git log entry : "Fixed the endless recursion, caused by the depth-first nature of the implementation; now, it first does a quick numberRule check on the ligands, which wil.."
14:43 maclean commit : 1d72a1a477308b2b3ffca9a51f55c03ce5f6e64c
14:43 egonw sounds good to me...
14:43 maclean I checked out a fresh branch from master and applied 160-cip on top.
14:44 maclean specifically "git pull egon_patches 160-cip" (I know, I know, don't pull)
14:44 egonw :)
14:44 egonw ok, this is the test code I used:
14:44 egonw oh, wait, I'll just push
14:44 maclean ok
14:46 egonw pull is only bad if you have local commits
14:46 egonw in 160-cip itself, and not in a branch...
14:46 egonw ok, patch online
14:47 maclean ack
14:48 maclean hmmm. terminates here too.
14:49 egonw so, two options:
14:49 egonw it matters how the data is inputted
14:49 egonw your input is bad
14:49 maclean did you look at the gist?
14:49 egonw but your SMILES output looked fine
14:49 egonw yeah, looking at that right now
14:49 egonw but your SMILES output looked fine
14:49 egonw so
14:49 egonw there must be a difference in how we provide the input
14:49 * egonw trying to get his head around 0, n, (n+1) etc
14:50 maclean sorry, yes.
14:50 maclean that's so that I can make rings of any size
14:50 maclean but such generalised creation code is hard to read
14:51 maclean (the worst being, when I made some code to make toroidal grid-graphs...)
14:51 egonw makeChiralRing(int n) looks fine to me
14:53 maclean well, I would think that the stereo element part is the critical bit...
14:55 maclean I guess it would be good to Assert that the atoms in the array passed to the tetrahedral chirality constructor are distinct.
14:56 egonw yeah, possible
14:56 egonw I'm converting your code into a unit test...
14:56 egonw will run and confirm the problem
14:56 egonw (or not)
14:57 maclean ok
14:58 egonw confirmed
14:58 egonw so, it has to do with the input
14:59 maclean which differs somehow from what the smiles parser is doing. odd.
15:04 maclean Oh, wait. The smiles parser does atom typing...hmmm.
15:05 egonw sure, but that's not important...
15:05 egonw or should not be...
15:05 egonw or ...
15:05 maclean yep - didn't work :)
15:06 maclean I thought it might affect the sorting by atom number.
15:06 egonw SMILES is not setting atom number or isotopic mass number
15:07 egonw moreover, it will look up that info if not set...
15:08 maclean ok
15:19 egonw ok, found the bug
15:19 egonw it's atom ordering
15:19 egonw stupid me
15:19 egonw thanx for catching this
15:20 egonw the current problem lies in CIPLigandRule lines 67-70
15:20 maclean n/p
15:20 egonw two issues really:
15:20 egonw no, actually not...
15:20 egonw just one issue
15:20 egonw one O takes precedence over two C's
15:21 egonw but the problem is that it should order any ligand series larger than one...
15:22 egonw that last patched fixed one thing, but broke another :(
15:22 egonw (peer review)++
15:27 egonw maclean: if you can cook up a jar and confirm it with your test code, that would be much appreciated
15:27 egonw fix is online now
15:27 maclean "cook up a jar"? Is that some kind of drug slang? :)
15:28 maclean Sorry - I do know how to make jarfiles, but of what? The signatures/cip stuff?
15:30 egonw yeah, of 160-cip
15:30 egonw ok, got to do some work in the garden now
15:30 maclean ok
15:33 egonw the two patches are now also attached in the patch report.
15:33 egonw bbl
17:24 egonw joined #cdk
17:54 maclean joined #cdk
18:17 egonw joined #cdk
22:27 slyrus_ joined #cdk

| Channels | #cdk index | Today | | Search | Google Search | Plain-Text | summary