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

Roll out new particle property getters in the Python interface #4461

Closed
jngrad opened this issue Mar 3, 2022 · 2 comments · Fixed by #4473
Closed

Roll out new particle property getters in the Python interface #4461

jngrad opened this issue Mar 3, 2022 · 2 comments · Fixed by #4473

Comments

@jngrad
Copy link
Member

jngrad commented Mar 3, 2022

The Python interface should reflect the recent changes to the Particle struct. The goal is to adapt all property getters to the new syntax, except for ext_flag and rotation which can be ignored (they rely on special getters outside the class definition). The setters don't need to change. This task is suitable for a coding day and can be done in about half an hour. No C++ knowledge required.

The src/python/espressomd/particle_data.pyx file currently looks like this:

cdef class ParticleHandle:
    property type:
        def __get__(self):
            self.update_particle_data()
            return self.particle_data.p.type

It should look like this:

cdef class ParticleHandle:
    property type:
        def __get__(self):
            self.update_particle_data()
            return self.particle_data.type()  # <<< now we simply call the struct method

To achieve this, the src/python/espressomd/particle_data.pxd needs to be changed from this:

cdef extern from "particle_data.hpp":
    ctypedef struct particle_properties "ParticleProperties":
        int    identity
        int    type

    ctypedef struct particle "Particle":
        particle_properties p
        Vector3d calc_dip()

to this:

cdef extern from "particle_data.hpp":
    ctypedef struct particle_properties "ParticleProperties":
        int    identity  # <<< 'type' is gone, 'identity' can be removed too

    ctypedef struct particle "Particle":
        particle_properties p
        Vector3d calc_dip()
        int type()  # <<< struct method is declared, do the same for all other properties

The same changes have to be applied to 2-3 lines in src/python/espressomd/visualization_mayavi.pyx.

Once all getters are replaced, particle_data.pxd should no longer need to declare the particle substructs.

@jngrad
Copy link
Member Author

jngrad commented Mar 4, 2022

Additional task: do the same for compiler-dependent getters. All particle properties should now be available, even when features are not compiled in (when in doubt, run the python testsuite with maintainer/configs/empty.hpp as myconfig.hpp). Go from:

cdef extern from "particle_data.hpp":
    ctypedef struct particle "Particle":
        particle_properties p
        Vector3d calc_dip()

    IF ROTATIONAL_INERTIA:
        void set_particle_rotational_inertia(int part, const Vector3d & rinertia)
        Vector3d get_particle_rotational_inertia(const particle * p)

To this (and adapt the .pyx file accordingly):

cdef extern from "particle_data.hpp":
    ctypedef struct particle "Particle":
        particle_properties p
        Vector3d calc_dip()
        Vector3d rinertia()  # <<< struct method is declared

    IF ROTATIONAL_INERTIA:
        void set_particle_rotational_inertia(int part, const Vector3d & rinertia)
        # old getter is gone

The getters that are removed from the Cython file can also be removed from src/core/particle_data.hpp.

@christophlohrmann
Copy link
Contributor

can do

@kodiakhq kodiakhq bot closed this as completed in #4473 Mar 14, 2022
kodiakhq bot added a commit that referenced this issue Mar 14, 2022
Fixes #4461 

Description of changes:
- Update all properties to the new getters except gamma and VS properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants