Extending domain model

Development
1

Hi,

First of all

In our group much time goes with building molecular geometry and preparing
the input files for the simulation tools in the “manual” way, so happy that
found your project. The tool seems to be a really big help, so thank for the
development efforts to the team and to all contributors… keep it up :slight_smile:
Now we would like to extend it a bit to match our needs, and there are some
questions…

And the issues are

[1] Extending atom properties easier

a) strange place of definition

We would like to add velocity and acceleration for the atoms beside the
position. I expected that I can do it in libavogadro/src/atom.h, instead it
is described in libavogadro/src/molecule.h. It is strange to my
object-oriented mind. I think it would be more logical to keep it in the
first. Does it have speed issues? So it is structured this way because
manipulating with positions is not so costly in the related operations?

b) grouping properties and code maintenance

For maintainibility point of view it would be better to make a struct or
class for the properties, this way extension would be very easy. Now have to
define each field separately and review the whole code where to change to
keep it consistent (resize, etc.). Is it intentionally so or it would be a
logical restructuring?

[2] Class field definition sequence counts?

Now I faced a weird error. When I started to extend the properties according
to the current concept, I put

libavogadro/src/molecule.h:807

std::vector<Eigen::Vector3d> *m_atomVelocity; // Atom velocity vector

right after

std::vector<Eigen::Vector3d> *m_atomPos; // Atom position vector

then after running the program it ended up in segmentation fault
systematically.

Now I put the new property at the end of the field list (line 823) and it
works fine.

How it can be? I can imagine only if very bad coding tricks were applied
operating with pointers on the dark side :slight_smile: but I don’t assume it. Might it
have a basic reason that I dont consider?

And all these were in Avogadro 1.0.1 under Ubuntu 9.10

I did not review the whole sources, so sorry if I put obvious questions.

Thanks and regards:

Mark

2

Hi,

On Wed, Jul 21, 2010 at 1:50 PM, Márk Visontai mark.visontai@gmail.com wrote:

Hi,

First of all

In our group much time goes with building molecular geometry and preparing
the input files for the simulation tools in the “manual” way, so happy that
found your project. The tool seems to be a really big help, so thank for the
development efforts to the team and to all contributors… keep it up :slight_smile:
Now we would like to extend it a bit to match our needs, and there are some
questions…

And the issues are

[1] Extending atom properties easier

a) strange place of definition

We would like to add velocity and acceleration for the atoms beside the
position. I expected that I can do it in libavogadro/src/atom.h, instead it
is described in libavogadro/src/molecule.h. It is strange to my
object-oriented mind. I think it would be more logical to keep it in the
first. Does it have speed issues? So it is structured this way because
manipulating with positions is not so costly in the related operations?

Performance is the main reason for doing this. In future versions this
method will probably be used even more. Since the atom has a pointer
to the molecule, you can add convenience functions to Atom to just
call the method in Molecule.

b) grouping properties and code maintenance

For maintainibility point of view it would be better to make a struct or
class for the properties, this way extension would be very easy. Now have to
define each field separately and review the whole code where to change to
keep it consistent (resize, etc.). Is it intentionally so or it would be a
logical restructuring?

This is a good idea IMHO.

[2] Class field definition sequence counts?

Now I faced a weird error. When I started to extend the properties according
to the current concept, I put

libavogadro/src/molecule.h:807

std::vector<Eigen::Vector3d> *m_atomVelocity; // Atom velocity vector

right after

std::vector<Eigen::Vector3d> *m_atomPos; // Atom position vector

then after running the program it ended up in segmentation fault
systematically.

This sounds like an ABI (Application Binary Interface) issue. Make
sure to run the correct executable and ensure it uses the correct
library. For example, if you have avogadro installed from the ubuntu
repository and have a custom build version, I recommend to remove the
ubuntu version.

Now I put the new property at the end of the field list (line 823) and it
works fine.

How it can be? I can imagine only if very bad coding tricks were applied
operating with pointers on the dark side :slight_smile: but I don’t assume it. Might it
have a basic reason that I dont consider?

And all these were in Avogadro 1.0.1 under Ubuntu 9.10

I did not review the whole sources, so sorry if I put obvious questions.

Thanks and regards:

Mark

This SF.net email is sponsored by Sprint
What will you do first with EVO, the first 4G phone?
Visit sprint.com/first – http://p.sf.net/sfu/sprint-com-first

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

3

On Wed, Jul 21, 2010 at 8:31 PM, Tim Vandermeersch
tim.vandermeersch@gmail.com wrote:

Hi,

On Wed, Jul 21, 2010 at 1:50 PM, Márk Visontai mark.visontai@gmail.com wrote:

Hi,

First of all

In our group much time goes with building molecular geometry and preparing
the input files for the simulation tools in the “manual” way, so happy that
found your project. The tool seems to be a really big help, so thank for the
development efforts to the team and to all contributors… keep it up :slight_smile:
Now we would like to extend it a bit to match our needs, and there are some
questions…

And the issues are

[1] Extending atom properties easier

a) strange place of definition

We would like to add velocity and acceleration for the atoms beside the
position. I expected that I can do it in libavogadro/src/atom.h, instead it
is described in libavogadro/src/molecule.h. It is strange to my
object-oriented mind. I think it would be more logical to keep it in the
first. Does it have speed issues? So it is structured this way because
manipulating with positions is not so costly in the related operations?

Performance is the main reason for doing this. In future versions this
method will probably be used even more. Since the atom has a pointer
to the molecule, you can add convenience functions to Atom to just
call the method in Molecule.

I forgot to say: For some atom properties you can use Qt’s properties
system. It allows you to store additional properties (QVariants) in
any QObject object. For non-performance critical properties this is an
ideal choice.

b) grouping properties and code maintenance

For maintainibility point of view it would be better to make a struct or
class for the properties, this way extension would be very easy. Now have to
define each field separately and review the whole code where to change to
keep it consistent (resize, etc.). Is it intentionally so or it would be a
logical restructuring?

This is a good idea IMHO.

[2] Class field definition sequence counts?

Now I faced a weird error. When I started to extend the properties according
to the current concept, I put

libavogadro/src/molecule.h:807

std::vector<Eigen::Vector3d> *m_atomVelocity; // Atom velocity vector

right after

std::vector<Eigen::Vector3d> *m_atomPos; // Atom position vector

then after running the program it ended up in segmentation fault
systematically.

This sounds like an ABI (Application Binary Interface) issue. Make
sure to run the correct executable and ensure it uses the correct
library. For example, if you have avogadro installed from the ubuntu
repository and have a custom build version, I recommend to remove the
ubuntu version.

Now I put the new property at the end of the field list (line 823) and it
works fine.

How it can be? I can imagine only if very bad coding tricks were applied
operating with pointers on the dark side :slight_smile: but I don’t assume it. Might it
have a basic reason that I dont consider?

And all these were in Avogadro 1.0.1 under Ubuntu 9.10

I did not review the whole sources, so sorry if I put obvious questions.

Thanks and regards:

Mark

This SF.net email is sponsored by Sprint
What will you do first with EVO, the first 4G phone?
Visit sprint.com/first – http://p.sf.net/sfu/sprint-com-first

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