[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [IMP-dev] IMP-dev Digest, Vol 13, Issue 5




On Nov 11, 2008, at 8:50 PM, Javier Ángel Velázquez Muriel wrote:

Hi, some comments here:

1) I am writing the 2D classes for vector, matrix , image, and image
headers these days. image and imageheader will be part of the em
module. the others,
wherever you suggest. If you plan to do something similar, Keren, let me know.
Sure Javi - it might make sense to reuse all of the 3D stuff we have - with some base classes that the 3D and 3D inherit from. If you like to discuss it further it might be useful to post the header files.

2) It doesn't seem  logical to me why transformations, rotations and
matrices are not part of the kernel, or at least of a math module.
Misc is an inappropriate module.

misc is a temporary solution until Ben will come back, as I do not have write permissions to some of the modules.

2008/11/11  <>:
Send IMP-dev mailing list submissions to
      

To subscribe or unsubscribe via the World Wide Web, visit
      https://salilab.org/mailman/listinfo/imp-dev
or, via email, send a message with subject or body 'help' to
      

You can reach the person managing the list at
      

When replying, please edit your Subject line so it is more specific
than "Re: Contents of IMP-dev digest..."


Today's Topics:

 1. Comments on transformation stuff (Daniel Russel)
 2. Re: Comments on transformation stuff (Keren Lasker)
 3. Re: Comments on transformation stuff (Daniel Russel)


----------------------------------------------------------------------

Message: 1
Date: Tue, 11 Nov 2008 05:58:36 -0800
From: Daniel Russel <>
Subject: [IMP-dev] Comments on transformation stuff
To: List for IMP development <>
Message-ID: <>
Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes

Rotation3D:
- the protected methods should probably be private as nothing is
likely to inherit from Rotation3D and doxygen skips documenting
private methods and the methods are not general purpose
- it would be nice to be able to initialize from a vector and an angle - the current constructor should probably say Euler angle somewhere in
its docs.

Matrix3D:
- why are there the rotation methods there? A cleaner interface would
probably be to have a method is_rotation() and another get_rotation()
so you can check is is close to a rotation and then get the rotation3D
object out directly. Having to go through intermediates of some sort
is messy.
- it needs an operator[] which returns a vector3D (with appropriate
bounds checking and all that).
- I would suggest using \brief to define multi-line brief comments

ParticleFunction:
- Why does ParticleFunction take Particles? If you want that it should be ParticlesFunction, but it seems to me what we really want is for it
to only take one particle and then use apply methods if we want to
apply it to more than one. Otherwise we will be creating Particles
with one Particle everywhere.
- apply should be pure virtual so that you can tell if you got the
name or signature wrong when overriding it.
- at least one of the methods needs to be defined in a .cpp to get
linking and casting right when using multiple libraries


------------------------------

Message: 2
Date: Tue, 11 Nov 2008 16:12:03 -0500
From: Keren Lasker <>
Subject: Re: [IMP-dev] Comments on transformation stuff
To: List for IMP development <>
Message-ID: <>
Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes

Daniel  - thanks for your good review!
I'll make the suggested changes.
I agree with everything on Rotation3D and Matrix3D.
Regarding ParticleFunction - see discussion below.
On Nov 11, 2008, at 8:58 AM, Daniel Russel wrote:

Rotation3D:
- the protected methods should probably be private as nothing is
likely to inherit from Rotation3D and doxygen skips documenting
private methods and the methods are not general purpose

- it would be nice to be able to initialize from a vector and an angle - the current constructor should probably say Euler angle somewhere in
its docs.




Matrix3D:
- why are there the rotation methods there? A cleaner interface would probably be to have a method is_rotation() and another get_rotation() so you can check is is close to a rotation and then get the rotation3D
object out directly. Having to go through intermediates of some sort
is messy.
- it needs an operator[] which returns a vector3D (with appropriate
bounds checking and all that).
- I would suggest using \brief to define multi-line brief comments

ParticleFunction:
- Why does ParticleFunction take Particles? If you want that it should be ParticlesFunction, but it seems to me what we really want is for it
to only take one particle and then use apply methods if we want to
apply it to more than one. Otherwise we will be creating Particles
with one Particle everywhere.
yes ... , but then I'll have to do a loop each time I want to rotate a
molecule.
maybe it would be better to have MolecularHierarchyDecoratorFunction ?
So I could just give as input the root of the molecule?

- apply should be pure virtual so that you can tell if you got the
name or signature wrong when overriding it.
sure.

- at least one of the methods needs to be defined in a .cpp to get
linking and casting right when using multiple libraries
For some reason python can not find the function implementation if it
is in the cpp file. It happened to Joerg as well - is there anything
else other than updating the include/Sconscript, ,src/Sconscript and
pyext/misc.i ?

_______________________________________________
IMP-dev mailing list

https://salilab.org/mailman/listinfo/imp-dev



------------------------------

Message: 3
Date: Tue, 11 Nov 2008 06:21:15 -0800
From: Daniel Russel <>
Subject: Re: [IMP-dev] Comments on transformation stuff
To: List for IMP development <>
Message-ID: <>
Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes

ParticleFunction:
- Why does ParticleFunction take Particles? If you want that it
should
be ParticlesFunction, but it seems to me what we really want is for
it
to only take one particle and then use apply methods if we want to
apply it to more than one. Otherwise we will be creating Particles
with one Particle everywhere.
yes ... , but then I'll have to do a loop each time I want to rotate a
molecule.
Write a method to apply it to a molecule. And another to apply it to a
Particles.

maybe it would be better to have MolecularHierarchyDecoratorFunction ?
So I could just give as input the root of the molecule?
If it takes a single particle it is easy enough to have a method which traverses the hierarchy and applies it to each atom, for example. This
is annoying to do with the current definition.




- apply should be pure virtual so that you can tell if you got the
name or signature wrong when overriding it.
sure.

- at least one of the methods needs to be defined in a .cpp to get
linking and casting right when using multiple libraries
For some reason python can not find the function implementation if it
is in the cpp file. It happened to Joerg as well - is there anything
else other than updating the include/Sconscript, ,src/Sconscript and
pyext/misc.i ?
I'm not quite sure what you mean. What exactly happens? And what were
you doing? See UnaryFunction as an example of how it should be.

As it stands now, each library which uses ParticleFunction will have
its own definition of the base class and they will not be
interconvertible (i.e. a ParticleFunction defined in IMP.core cannot
be used in something in Kernel which expects a particlefunction).


------------------------------

_______________________________________________
IMP-dev mailing list

https://salilab.org/mailman/listinfo/imp-dev


End of IMP-dev Digest, Vol 13, Issue 5
**************************************

_______________________________________________
IMP-dev mailing list

https://salilab.org/mailman/listinfo/imp-dev