Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

md.constrain.Rigid uses charges and diameters only in create_bodies #1350

Closed
joaander opened this issue Jun 20, 2022 · 3 comments
Closed

md.constrain.Rigid uses charges and diameters only in create_bodies #1350

joaander opened this issue Jun 20, 2022 · 3 comments
Labels
breaking Changes that will break API. bug Something isn't working essential Work that must be completed.

Comments

@joaander
Copy link
Member

Description

In undocumented behavior, md.constrain.Rigid uses charges and diameters only in create_bodies. This is unlike constituent_types, which is used checked the body validation process and positions and orientation which are applied every time step.

Also, changing only the charges or diameters is not counted as a changed body (e.g. making a change to charges only does not trigger a new validation):

if (type[i] != h_body_type.data[body_index] || pos[i] != h_body_pos.data[body_index]
|| orientation[i] != h_body_orientation.data[body_index])
{
body_updated = true;
}

Script

# Include a minimal script that reproduces the problem
import hoomd

snapshot = hoomd.Snapshot()
snapshot.configuration.box = (10,10,10,0,0,0)
snapshot.particles.N = 2
snapshot.particles.position[:] = [(0, 0, 0), (1, 0, 0)]
snapshot.particles.types = ['A', 'B']
snapshot.particles.typeid[:] = [0, 1]
snapshot.particles.charge[:] = [-1.0, 1.0]
snapshot.particles.diameter[:] = [1.0, 2.0]
snapshot.particles.body[:] = [0.0, 0.0]

sim = hoomd.Simulation(device=hoomd.device.CPU(), seed=10)
sim.create_state_from_snapshot(snapshot)

rigid = hoomd.md.constrain.Rigid()
rigid.body['A'] = {
    "constituent_types": ['B'],
    "positions": [(0,0,1)],
    "orientations": [(1.0, 0.0, 0.0, 0.0)],
    "charges": [0.0],
    "diameters": [1.0]
    }

integrator = hoomd.md.Integrator(dt=0, integrate_rotational_dof=True)
integrator.rigid = rigid
sim.operations.integrator = integrator

sim.run(0)

Output

no output

Expected output

I expected an error that the charge and/or diameter given in rigid.body['A'] fails to match that in the simulation state. Like this error given when constituent_types doesn't match:

RuntimeError: Error validating rigid bodies: Constituent particle types must be consistent with the rigid body definition.  Rigid body definition is in order of particle tag.

ALTERNATIVELY, I expect the API to not accept charges and diameters as body type parameters (see discussion below).

Configuration

Versions:

  • Python version: all
  • HOOMD-blue version: 3.2.0

Developer

Before implementing a fix, we should discuss what the correct API should be. Many users have expressed extreme confusion over the charge and diameter fields. There is less confusion over constituent_types, however it has the same behavior (used for create_bodies and validation only).

I see two rational options:

  • Validate charges and diameters the same way as constituent_types. Document how these are used.
  • Remove charges, diameters and consituent_types from the bodies type parameter and make them parameters of create_bodies (optional in the case of charges and diameters). Document how these are used.

The 2nd option will involve a deprecation or a breaking change. However, it cleans up the API as it longer requires that the user provide the same information in two places (simulation state and body definition). Users will still be annoyed that they need to define the local positions and orientations, but that is not avoidable as the data available in the simulation state is both not sufficient and overdetermined.

@joaander joaander added the bug Something isn't working label Jun 20, 2022
@b-butler
Copy link
Member

I would suggest the 2nd as charges and diameters at least make most sense just in the simulation. I am not sure for constituent_types, but it does provide the most flexibility that way (e.g. types could change mid simulation without error).

@joaander
Copy link
Member Author

Yes, I agree that we should remove constituent_types from the body type parameter as well. It removes all redundant information. We can discuss this further in the meeting tomorrow.

Note: if we choose to make this a breaking change, we can remove diameters altogether from HOOMD in v4.

@joaander joaander added breaking Changes that will break API. essential Work that must be completed. labels Aug 12, 2022
@joaander
Copy link
Member Author

As discussed in glotzerlab/hoomd-examples#73, we also need to modify HOOMD to enforce that the particle type ids match those in the body definition, as they are used when determining the per-type ghost width.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Changes that will break API. bug Something isn't working essential Work that must be completed.
Projects
None yet
Development

No branches or pull requests

2 participants