Question about Molecule::setAllConformers()

Hi all,

Currently Molecule::setAllConformers() will remove all conformers
except the first, then add on the new conformers. So, if a list of n
conformers is passed, the molecule ends up with n+1 conformers. This
is very counter-intuitive, IMO, and I’d like to change it so that
setAllConformers will set all conformers.

Here’s what I’m trying to do:

  • Generate a list of all conformers in a molecule
  • Sort and display the conformers by energy
  • Pick conformers to use later based on an energy-weighted probability

So, I’m:

  • Using the OBForceFields to generate a list of conformers as
    a std::vector<std::vectorEigen::Vector3d*> vector
  • Calling setAllConformers with the vector
  • Sorting and assigning probabilities

Having the 0th conformer remain is throwing off my probability
distribution. Another solution would be to add a
Molecule::removeConformer(uint ind), since there is currently no way
to delete the 0th conformer.

Any preferences from the community about how this should be handled?

Thanks,

Dave

On Sat, Aug 8, 2009 at 5:23 PM, David C. Lonieloniedavid@gmail.com wrote:

Hi all,

Currently Molecule::setAllConformers() will remove all conformers
except the first, then add on the new conformers. So, if a list of n
conformers is passed, the molecule ends up with n+1 conformers. This
is very counter-intuitive, IMO, and I’d like to change it so that
setAllConformers will set all conformers.

Replacing all conformers is the right way I think. (taking the
comment below in regards: if setConformers is called with 0
conformers, we keep only one…)

Here’s what I’m trying to do:

  • Generate a list of all conformers in a molecule
  • Sort and display the conformers by energy
  • Pick conformers to use later based on an energy-weighted probability

So, I’m:

  • Using the OBForceFields to generate a list of conformers as
    a std::vector<std::vectorEigen::Vector3d*> vector
  • Calling setAllConformers with the vector
  • Sorting and assigning probabilities

Having the 0th conformer remain is throwing off my probability
distribution. Another solution would be to add a
Molecule::removeConformer(uint ind), since there is currently no way
to delete the 0th conformer.

We should always have at least one conformer to display… This is
probably why setConformers handles the 1st one as a special case.

Tim

Any preferences from the community about how this should be handled?

Thanks,

Dave


Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what’s new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july


Avogadro-devel mailing list
Avogadro-devel@lists.sourceforge.net
avogadro-devel List Signup and Options

On Saturday 08 August 2009 11:35:57 Tim Vandermeersch wrote:

On Sat, Aug 8, 2009 at 5:23 PM, David C. Lonieloniedavid@gmail.com wrote:

Hi all,

Currently Molecule::setAllConformers() will remove all conformers
except the first, then add on the new conformers. So, if a list of n
conformers is passed, the molecule ends up with n+1 conformers. This
is very counter-intuitive, IMO, and I’d like to change it so that
setAllConformers will set all conformers.

Replacing all conformers is the right way I think. (taking the
comment below in regards: if setConformers is called with 0
conformers, we keep only one…)

This sounds like the behaviour one would expect, and so we should fix this.

Here’s what I’m trying to do:

  • Generate a list of all conformers in a molecule
  • Sort and display the conformers by energy
  • Pick conformers to use later based on an energy-weighted probability

So, I’m:

  • Using the OBForceFields to generate a list of conformers as
    a std::vector<std::vectorEigen::Vector3d*> vector
  • Calling setAllConformers with the vector
  • Sorting and assigning probabilities

Having the 0th conformer remain is throwing off my probability
distribution. Another solution would be to add a
Molecule::removeConformer(uint ind), since there is currently no way
to delete the 0th conformer.

We should always have at least one conformer to display… This is
probably why setConformers handles the 1st one as a special case.

The problem is that a molecule with zero conformers is a totally invalid state
to be in. There is an inherent assumption in a large amount of our code that
the molecule will have coordinates, even if they are all zero. What should be
drawn if there is no zeroth conformer? Should the render function just bail
out?

I am open to ideas, it would be good to keep our molecule class as general as
possible, whilst still being optimised for editing and rendering molecules. I
think it should be possible to replace the zeroth conformer, but I am not sure
whether it is useful to be able to remove it. Would zeroing this conformer be
helpful?

Marcus

On Sat, Sep 5, 2009 at 9:05 PM, Marcus D. Hanwellmarcus@cryos.org wrote:

On Saturday 08 August 2009 11:35:57 Tim Vandermeersch wrote:

On Sat, Aug 8, 2009 at 5:23 PM, David C. Lonieloniedavid@gmail.com wrote:

Hi all,

Currently Molecule::setAllConformers() will remove all conformers
except the first, then add on the new conformers. So, if a list of n
conformers is passed, the molecule ends up with n+1 conformers. This
is very counter-intuitive, IMO, and I’d like to change it so that
setAllConformers will set all conformers.

Replacing all conformers is the right way I think. (taking the
comment below in regards: if setConformers is called with 0
conformers, we keep only one…)

This sounds like the behaviour one would expect, and so we should fix this.

Here’s what I’m trying to do:

  • Generate a list of all conformers in a molecule
  • Sort and display the conformers by energy
  • Pick conformers to use later based on an energy-weighted probability

So, I’m:

  • Using the OBForceFields to generate a list of conformers as
    a std::vector<std::vectorEigen::Vector3d*> vector
  • Calling setAllConformers with the vector
  • Sorting and assigning probabilities

Having the 0th conformer remain is throwing off my probability
distribution. Another solution would be to add a
Molecule::removeConformer(uint ind), since there is currently no way
to delete the 0th conformer.

We should always have at least one conformer to display… This is
probably why setConformers handles the 1st one as a special case.

The problem is that a molecule with zero conformers is a totally invalid state
to be in. There is an inherent assumption in a large amount of our code that
the molecule will have coordinates, even if they are all zero. What should be
drawn if there is no zeroth conformer? Should the render function just bail
out?

I am open to ideas, it would be good to keep our molecule class as general as
possible, whilst still being optimised for editing and rendering molecules. I
think it should be possible to replace the zeroth conformer, but I am not sure
whether it is useful to be able to remove it. Would zeroing this conformer be
helpful?

Marcus

We should always have one conformer, this is not really a problem.
Trying to remove all (i.e. the last) conformers should not work. But
when setConformers is called all should be replaced. There is no
point in keeping the first around if there are n others available (it
looks ugly when looping animations too). So we should check if
conformers size in setConformers we and:

  1. size = 0 → create a single conformer with all zeros (just to make
    sure there is a valid conformer)
  2. size >= 1 → replace all conformers in the molecule with the new set

To put it differently: What should setConformers do?

current (very unintuitive): conformers = old[0] + new[0…n]
proposed above: conformers = new[0…n] (or zeros)

I have to go somewhere tonight but I’ll fix this tomorrow if you agree
with the proposed solution.

Cheers,
Tim