search

dmorse / simpatico

Molecular dynamics and Monte Carlo simulation package for polymeric and molecular liquids
GNU General Public License v3.0
31 stars 17 forks source link

Addition of new diagnostics in DdMd #5

Closed medapuram closed 12 years ago

dmorse commented 12 years ago

1) I think it would be clearer if the ...PairEnergies methods of PairPotential used a Matrix rather than 1D array to store and retrieve the elements. The use of a DArray relies on a convention for how the elements are ordered that needs to be explained, but which would be much more obvious is we just used a container with two elements (which just implements the mapping to a 1D array internally).

2) I don't understand the need for the transformCartToGen and transformGenToCart methds of the ConfigIo. The existing methods were designed to deal correctly with existing conventions for where in the integration loop the positions should be stored in Cartesian coordinates and where they will be stored in generalized coordinates. If they aren't, they should be. The AtomStorage class also provides analogous methods to transform coordinates in place, without writing to file, which is I think what you need to use for the DEFORM_CELL command.

medapuram commented 12 years ago

Dave,

  1. I agree that the matrix convention would be the more natural way for storing pair energies. I will change that right away.
  2. I will look into the already existing methods for use in the DEFORM_CELL command.

The extra 'if' statement was something I intended to remove. It was from an early draft of the code. I don't know how I missed it. I apologize.

Thanks, Pavani

On Thu, Aug 30, 2012 at 2:47 AM, David Morse notifications@github.comwrote:

1) I think it would be clearer if the ...PairEnergies methods of PairPotential used a Matrix rather than 1D array to store and retrieve the elements. The use of a DArray relies on a convention for how the elements are ordered that needs to be explained, but which would be much more obvious is we just used a container with two elements (which just implements the mapping to a 1D array internally).

2) I don't understand the need for the transformCartToGen and transformGenToCart methds of the ConfigIo. The existing methods were designed to deal correctly with existing conventions for where in the integration loop the positions should be stored in Cartesian coordinates and where they will be stored in generalized coordinates. If they aren't, they should be. The AtomStorage class also provides analogous methods to transform coordinates in place, without writing to file, which is I think what you need to use for the DEFORM_CELL command.

— Reply to this email directly or view it on GitHubhttps://github.com/dmorse/simpatico/pull/5#issuecomment-8151778.