SF.net SVN: avogadro: trunk/libavogadro/src/engines

On Feb 17, 2008, at 9:59 AM, cryosuk@users.sourceforge.net wrote:

Revert “Moved VDWSurface to IsoGen thread to make surface engine
more interactive” - makes the IsoGen class much less general and
breaks the orbital engine code.

Can we have a subclass for the VDW surfaces? It did seem to help the
surface engine interactivity.

Certainly we have a problem with premature optimization. For example,
we only do VDW or solvent-accessible surfaces. We don’t yet have code
to do solvent-excluded surfaces. And orbitals or other cube files will
have totally different surface structure.

Cheers,
-Geoff

On Feb 17, 2008 4:11 PM, Geoffrey Hutchison geoff.hutchison@gmail.com wrote:

On Feb 17, 2008, at 9:59 AM, cryosuk@users.sourceforge.net wrote:

Revert “Moved VDWSurface to IsoGen thread to make surface engine
more interactive” - makes the IsoGen class much less general and
breaks the orbital engine code.

Can we have a subclass for the VDW surfaces? It did seem to help the
surface engine interactivity.

Certainly we have a problem with premature optimization. For example,
we only do VDW or solvent-accessible surfaces. We don’t yet have code
to do solvent-excluded surfaces. And orbitals or other cube files will
have totally different surface structure.

I added an enumeration to the IsoGen class with the type of surface, I
was hoping that would be general enough. Altough only VDWsurfaceType
was implemented, other types such as ExcludedSurfaceType, OrbitalType,
GeneralGridType (with changeable IsoValue), … could be added later.
But we could make a separate class for VDWSurface(), but this seemed
more (unneeded) complex at the time.

On Sunday 17 February 2008 12:13:19 Tim Vandermeersch wrote:

On Feb 17, 2008 4:11 PM, Geoffrey Hutchison geoff.hutchison@gmail.com
wrote:

On Feb 17, 2008, at 9:59 AM, cryosuk@users.sourceforge.net wrote:

Revert “Moved VDWSurface to IsoGen thread to make surface engine
more interactive” - makes the IsoGen class much less general and
breaks the orbital engine code.

Can we have a subclass for the VDW surfaces? It did seem to help the
surface engine interactivity.

Certainly we have a problem with premature optimization. For example,
we only do VDW or solvent-accessible surfaces. We don’t yet have code
to do solvent-excluded surfaces. And orbitals or other cube files will
have totally different surface structure.

I added an enumeration to the IsoGen class with the type of surface, I
was hoping that would be general enough. Altough only VDWsurfaceType
was implemented, other types such as ExcludedSurfaceType, OrbitalType,
GeneralGridType (with changeable IsoValue), … could be added later.
But we could make a separate class for VDWSurface(), but this seemed
more (unneeded) complex at the time.

My reversion was possibly a little premature? The way I saw it was that we
should have a specialised IsoGen class that is very general and just deals
with cubes and generates triangles of an isosurface, then we would generate
these cubes in various other ways such as loading from files etc.

I think that the VdW cube should be generated by its own class or the surface
engine, cached and then regenerated upon modification to the molecule. We
already have the necessary signals implemented in the engines for just this
purpose.

I am not sure I see the motivation for putting the cube generation code into
the IsoGen class other than to use the threading features of the IsoGen
class. I am certainly open to alternate approaches though.

On Feb 17, 2008 6:34 PM, Marcus D. Hanwell marcus@cryos.org wrote:

On Sunday 17 February 2008 12:13:19 Tim Vandermeersch wrote:

On Feb 17, 2008 4:11 PM, Geoffrey Hutchison geoff.hutchison@gmail.com
wrote:

On Feb 17, 2008, at 9:59 AM, cryosuk@users.sourceforge.net wrote:

Revert “Moved VDWSurface to IsoGen thread to make surface engine
more interactive” - makes the IsoGen class much less general and
breaks the orbital engine code.

Can we have a subclass for the VDW surfaces? It did seem to help the
surface engine interactivity.

Certainly we have a problem with premature optimization. For example,
we only do VDW or solvent-accessible surfaces. We don’t yet have code
to do solvent-excluded surfaces. And orbitals or other cube files will
have totally different surface structure.

I added an enumeration to the IsoGen class with the type of surface, I
was hoping that would be general enough. Altough only VDWsurfaceType
was implemented, other types such as ExcludedSurfaceType, OrbitalType,
GeneralGridType (with changeable IsoValue), … could be added later.
But we could make a separate class for VDWSurface(), but this seemed
more (unneeded) complex at the time.

My reversion was possibly a little premature? The way I saw it was that we
should have a specialised IsoGen class that is very general and just deals
with cubes and generates triangles of an isosurface, then we would generate
these cubes in various other ways such as loading from files etc.

I think that the VdW cube should be generated by its own class or the surface
engine, cached and then regenerated upon modification to the molecule. We
already have the necessary signals implemented in the engines for just this
purpose.

I am not sure I see the motivation for putting the cube generation code into
the IsoGen class other than to use the threading features of the IsoGen
class. I am certainly open to alternate approaches though.

I moved it so it would be in a thread, for larger molecules having
VDWSurface inside the engine (no thread) doesn’t work very well. This
was my only motivation. Even if we cache the surface, the initial call
to VDWSurface would slow everything down. But giving VDWSurface it’s
own thread is a good approach where we keep the IsoGen class very
general.

Tim

On Sunday 17 February 2008 13:54:59 Tim Vandermeersch wrote:

On Feb 17, 2008 6:34 PM, Marcus D. Hanwell marcus@cryos.org wrote:

I am not sure I see the motivation for putting the cube generation code
into the IsoGen class other than to use the threading features of the
IsoGen class. I am certainly open to alternate approaches though.

I moved it so it would be in a thread, for larger molecules having
VDWSurface inside the engine (no thread) doesn’t work very well. This
was my only motivation. Even if we cache the surface, the initial call
to VDWSurface would slow everything down. But giving VDWSurface it’s
own thread is a good approach where we keep the IsoGen class very
general.

Totally agree and I think giving it its own thread is the right way to go
along with caching as the isosurface will not change unless the molecule is
modified - infrequent… The orbital stuff is now working too which I thought
was good news so we have two users of the IsoGen class.

I will work on improving the caching performance and making things a little
more general still - the IsoGen class still needs some tweaking to make it
more configurable and to cache its own results. We should save the VdW grid
too. I will see what I can do later unless you would like to do it?