Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#651 AllCycles/AllRingsFinder

Accepted
closed
nobody
master
1
2013-08-01
2013-06-28
John May
No

Okay split this in to two braches...

  • [feature/all-cycles] is the new implementation (working on a 'graph') - I also modified the existing cycles so they all give the same representation. That is they start and end with the same vertex {0, 1, 2, 3, 4, 5, 0} = 6 member ring. A couple of tests had to be changed but it's better to be consistent. first commit - a26c853
  • [feature/improved-all-rings-finder] use the new implementation to replace the AllRingsFinder version. I've kept the API the same (except of an Integer to int conversion but that is minimal). I was going to wait and do all the ring sets (i.e. for the other cycle perception) at once but decided this was quite important. The way the conversion is done might changed but that's class private so we can do that it future. first commit - 1e7d94f

Oh and needs the guava patch.

Discussion

  • John May
    John May
    2013-06-28

    Oh and the timings are still good, there's a bit of lag with the ring conversion but it's minimal and still offers a massive improvement.

     
  • Thanx, I'll try to review this asap. (but don't let that stop others of reviewing it before me)

     
  • John May
    John May
    2013-06-28

    Would it be possible skim through the module shuffling stuff first - 644. Don't need a full review but I'd already made the pdbcml test suite as part of that patch and seems a bit silly if we both patch the same things.

    I do need to break it up (in particular the ioformats I need to think more about) but there some stuff in that's holding me up from fixing other stuff/bugs - ala smiles.

     
    • status: open --> closed
    • Group: Needs_Review --> Accepted
     
  • Closed. Looks good.

     
    • status: closed --> open
    • Group: Accepted --> Needs_Review
     
  • Oops... premature... forgot the second branch...

     
  • John May
    John May
    2013-08-01

    Okay, I'll remove the ignored tests and what would you like to do about the deprecated 'boolean' option for logger?

    J

    On 1 Aug 2013, at 16:29, Egon Willighagen egonw@users.sf.net wrote:

    Second branch looks good too, except for a few minor comments... https://github.com/johnmay/cdk/commits/feature/improved-all-rings-finder

    Gonna apply it to master now.

    [patches:#651] AllCycles/AllRingsFinder

    Status: open
    Labels: rings AllRingsFinder
    Created: Fri Jun 28, 2013 10:54 AM UTC by John May
    Last Updated: Thu Aug 01, 2013 02:29 PM UTC
    Owner: nobody

    Okay split this in to two braches...

    [feature/all-cycles] is the new implementation (working on a 'graph') - I also modified the existing cycles so they all give the same representation. That is they start and end with the same vertex {0, 1, 2, 3, 4, 5, 0} = 6 member ring. A couple of tests had to be changed but it's better to be consistent. first commit - a26c853
    [feature/improved-all-rings-finder] use the new implementation to replace the AllRingsFinder version. I've kept the API the same (except of an Integer to int conversion but that is minimal). I was going to wait and do all the ring sets (i.e. for the other cycle perception) at once but decided this was quite important. The way the conversion is done might changed but that's class private so we can do that it future. first commit - 1e7d94f
    Oh and needs the guava patch.

    Sent from sourceforge.net because cdk-patches@lists.sourceforge.net is subscribed to https://sourceforge.net/p/cdk/patches/

    To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/cdk/admin/patches/options. Or, if this is a mailing list, you can unsubscribe from the mailing list.


    Get your SQL database under version control now!
    Version control is standard for application code, but databases havent
    caught up. So what steps can you take to put your SQL databases under
    version control? Why should you start doing it? Read more to find out.
    http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk_______
    Cdk-patches mailing list
    Cdk-patches@lists.sourceforge.net
    https://lists.sourceforge.net/lists/listinfo/cdk-patches

     
  • Please remove it.

    Egon

    On Thu, Aug 1, 2013 at 5:55 PM, John May jwmay@users.sf.net wrote:

    Okay, I'll remove the ignored tests and what would you like to do about the
    deprecated 'boolean' option for logger?

    J

    On 1 Aug 2013, at 16:29, Egon Willighagen egonw@users.sf.net wrote:

    Second branch looks good too, except for a few minor comments...
    https://github.com/johnmay/cdk/commits/feature/improved-all-rings-finder

    Gonna apply it to master now.

    [patches:#651] AllCycles/AllRingsFinder

    Status: open
    Labels: rings AllRingsFinder
    Created: Fri Jun 28, 2013 10:54 AM UTC by John May
    Last Updated: Thu Aug 01, 2013 02:29 PM UTC
    Owner: nobody

    Okay split this in to two braches...

    [feature/all-cycles] is the new implementation (working on a 'graph') - I
    also modified the existing cycles so they all give the same representation.
    That is they start and end with the same vertex {0, 1, 2, 3, 4, 5, 0} = 6
    member ring. A couple of tests had to be changed but it's better to be
    consistent. first commit - a26c853
    [feature/improved-all-rings-finder] use the new implementation to replace
    the AllRingsFinder version. I've kept the API the same (except of an Integer
    to int conversion but that is minimal). I was going to wait and do all the
    ring sets (i.e. for the other cycle perception) at once but decided this was
    quite important. The way the conversion is done might changed but that's
    class private so we can do that it future. first commit - 1e7d94f
    Oh and needs the guava patch.

    Sent from sourceforge.net because cdk-patches@lists.sourceforge.net is
    subscribed to https://sourceforge.net/p/cdk/patches/

    To unsubscribe from further messages, a project admin can change settings at
    https://sourceforge.net/p/cdk/admin/patches/options. Or, if this is a
    mailing list, you can unsubscribe from the mailing list.


    Get your SQL database under version control now!
    Version control is standard for application code, but databases havent
    caught up. So what steps can you take to put your SQL databases under
    version control? Why should you start doing it? Read more to find out.
    http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk_
    Cdk-patches mailing list
    Cdk-patches@lists.sourceforge.net
    https://lists.sourceforge.net/lists/listinfo/cdk-patches


    [patches:#651] AllCycles/AllRingsFinder

    Status: open
    Labels: rings AllRingsFinder
    Created: Fri Jun 28, 2013 10:54 AM UTC by John May
    Last Updated: Thu Aug 01, 2013 03:29 PM UTC
    Owner: nobody

    Okay split this in to two braches...

    [feature/all-cycles] is the new implementation (working on a 'graph') - I
    also modified the existing cycles so they all give the same representation.
    That is they start and end with the same vertex {0, 1, 2, 3, 4, 5, 0} = 6
    member ring. A couple of tests had to be changed but it's better to be
    consistent. first commit - a26c853
    [feature/improved-all-rings-finder] use the new implementation to replace
    the AllRingsFinder version. I've kept the API the same (except of an Integer
    to int conversion but that is minimal). I was going to wait and do all the
    ring sets (i.e. for the other cycle perception) at once but decided this was
    quite important. The way the conversion is done might changed but that's
    class private so we can do that it future. first commit - 1e7d94f

    Oh and needs the guava patch.


    Sent from sourceforge.net because you indicated interest in
    https://sourceforge.net/p/cdk/patches/651/

    To unsubscribe from further messages, please visit
    https://sourceforge.net/auth/subscriptions/

    --
    Dr E.L. Willighagen
    Postdoctoral Researcher
    Department of Bioinformatics - BiGCaT
    Maastricht University (http://www.bigcat.unimaas.nl/)
    Homepage: http://egonw.github.com/
    LinkedIn: http://se.linkedin.com/in/egonw
    Blog: http://chem-bla-ics.blogspot.com/
    PubList: http://www.citeulike.org/user/egonw/tag/papers

     
    • status: open --> closed
    • Group: Needs_Review --> Accepted
     
  • Second part pushed too now.