Marker trajectories (persistent markers)

[EveCharbie]

Answering to Issue #72

---

This change is 

[Ipuch]

@Ipuch reviewed 9 of 14 files at r1, all commit messages.
Reviewable status: 9 of 14 files reviewed, 29 unresolved discussions

---

pyorerun/model_interfaces/osim_model_interface.py line 239 at r1 (raw file):

                [mark.getLocationInGround(self.state).to_numpy() for mark in self.model.getMarkerSet()]
            )
        return markers_position

This method should disappear

Code quote:

    def persistent_markers(self, q: np.ndarray, frame_range: range) -> np.ndarray:
        """
        Returns a [N_markers x 3] array containing the position of each marker in the global reference frame
        """
        markers_position = np.zeros((self.nb_markers, 3, len(frame_range)))
        for frame in frame_range:
            self._update_kinematics(q[frame])
            markers_position[:, :, frame] = np.array(
                [mark.getLocationInGround(self.state).to_numpy() for mark in self.model.getMarkerSet()]
            )
        return markers_position

---

pyorerun/model_interfaces/abstract_model_interface.py line 92 at r1 (raw file):

    @abstractmethod
    def persistent_markers(self, q: np.ndarray, frame_range: range) -> np.ndarray:

I think the model, should be called only in a one-frame manner. This method should disappear

---

pyorerun/model_components/model_markers.py line 66 at r1 (raw file):

class PersistentMarkersUpdater(MarkersUpdater):

missing methods

def to_rerun(self, q: np.ndarray) -> None:
rr.log(
self.name,
self.to_component(q),
)

The "to_component" should be re-written to stack all the frames in q.shape[1]
def to_component(self, q: np.ndarray) -> rr.Points3D:
return rr.Points3D(
positions=self.callable_markers(q),
radii=self.marker_properties.radius_to_rerun(),
colors=self.marker_properties.color_to_rerun(),
labels=self.marker_properties.markers_names,
show_labels=self.marker_properties.show_labels_to_rerun(),
)

Code quote:

class PersistentMarkersUpdater(MarkersUpdater):

---

pyorerun/model_components/model_markers.py line 71 at r1 (raw file):

        name,
        marker_properties: MarkerProperties,
        callable_markers: callable,

the callable should already be sliced to the right size.

---

pyorerun/model_components/model_markers.py line 72 at r1 (raw file):

        marker_properties: MarkerProperties,
        callable_markers: callable,
        marker_trajectories: MarkerTrajectories,

the only extra arg should be the frame_range

---

pyorerun/model_components/model_markers.py line 74 at r1 (raw file):

        marker_trajectories: MarkerTrajectories,
    ):
        super().__init__(name, marker_properties, callable_markers)

yes for the super !

---

pyorerun/model_components/model_markers.py line 75 at r1 (raw file):

    ):
        super().__init__(name, marker_properties, callable_markers)
        self.marker_trajectories = marker_trajectories

to replace by self.frame_range

---

pyorerun/model_components/model_markers.py line 79 at r1 (raw file):

    @property
    def nb_marker_to_keep(self) -> int:
        return len(self.marker_trajectories.marker_names)

This should not exist in the class

Code quote:

    @property
    def nb_marker_to_keep(self) -> int:
        return len(self.marker_trajectories.marker_names)

---

pyorerun/model_components/model_markers.py line 85 at r1 (raw file):

        for i_frame, frame in enumerate(frames):
            markers[:, :, i_frame] = markers_to_keep[:, :, frame]
        return markers

I think this should be somehow displaced to the model to get a callable restrained to the considered markers.

Code quote:

    def compute_markers_to_display(self, markers_to_keep: np.ndarray, frames: list[int]) -> np.ndarray:
        markers = np.zeros((3, self.nb_marker_to_keep, len(frames)))
        for i_frame, frame in enumerate(frames):
            markers[:, :, i_frame] = markers_to_keep[:, :, frame]
        return markers

---

pyorerun/abstract/abstract_class.py line 30 at r1 (raw file):

class PersistentComponent(ABC):

we should have an extra method I guess:
range or frame_range

---

pyorerun/abstract/abstract_class.py line 32 at r1 (raw file):

class PersistentComponent(ABC):
    @abstractmethod
    def to_rerun(self, q: np.ndarray, frame_bounds: tuple[int, int]):

frame_range

but not sure, if it is the right behavior we should have yet.

---

pyorerun/__init__.py line 31 at r1 (raw file):

from .rrc3d import rrc3d as c3d
from .xp_components.timeseries_q import OsimTimeSeries
from .xp_components.marker_trajectories import MarkerTrajectories

PersistentMarkers

---

pyorerun/model_phase.py line 6 at r1 (raw file):

from .model_components.model_updapter import ModelUpdater
from .model_interfaces import AbstractModel
from .xp_components.marker_trajectories import MarkerTrajectories

PersistentMarkers

---

pyorerun/model_phase.py line 38 at r1 (raw file):

        tracked_markers: np.ndarray = None,
        muscle_colors: np.ndarray = None,
        marker_trajectories: MarkerTrajectories = None,

PersistentMarkers

---

pyorerun/model_phase.py line 46 at r1 (raw file):

                model=model,
                muscle_colors=muscle_colors,
                marker_trajectories=marker_trajectories,

no extra_argument

---

examples/biorbd/msk_model.py line 30 at r1 (raw file):

    # Example of how to add a persistent marker
    marker_trajectory = MarkerTrajectories(marker_names=["ULNA"], nb_frames=20)

I think I would store this information in model.options.persistent_markers=PersistentMarkers(name=["ULNA"], nb_frames=20)

---

examples/biorbd/msk_model.py line 34 at r1 (raw file):

    # viz.add_animated_model(model, q, display_q=True)
    viz.add_animated_model(model, q, display_q=False, marker_trajectories=marker_trajectory)

so this new extra argument will be gone.

---

pyorerun/model_interfaces/biorbd_model_interface.py line 131 at r1 (raw file):

            for i in range(self.nb_markers):
                markers_position[i, :, frame] = self.model.markers(GeneralizedCoordinates(q[frame]))[i].to_array()
        return markers_position

This method should disappear

Code quote:

    def persistent_markers(self, q: np.ndarray, frame_range: range) -> np.ndarray:
        """
        Returns a [N_markers x 3 x N_frames] array containing the position of each marker in the global reference frame over time
        """
        markers_position = np.zeros((self.nb_markers, 3, len(frame_range)))
        for frame in frame_range:
            for i in range(self.nb_markers):
                markers_position[i, :, frame] = self.model.markers(GeneralizedCoordinates(q[frame]))[i].to_array()
        return markers_position

---

pyorerun/model_components/model_updapter.py line 26 at r1 (raw file):

        muscle_colors: np.ndarray = None,
        marker_trajectories: MarkerTrajectories = None,
    ):

delete too

---

pyorerun/model_components/model_updapter.py line 222 at r1 (raw file):

                ),
                marker_trajectories=marker_trajectories,
                callable_markers=self.model.markers,

by using partial, you can partially "feed" some inputs without having to execute it.

callable_markers=partial(self.model.markers, idx=marker_idx)

Thus, it would require to update the interface of models, biorbd and opensim i guess

---

pyorerun/model_components/model_updapter.py line 244 at r1 (raw file):

            self.ligaments,
            self.muscles,
            self.persistent_markers,

I think this is safer, to create a new methods for persistent_components()

---

pyorerun/model_components/model_updapter.py line 264 at r1 (raw file):

        for component in self.components:
            component.to_rerun(q)

We need to add an extra loop

for pcomponent in self.persistent_components:
pcomponent.to_rerun(q:np.dnarray)

---

pyorerun/model_components/model_updapter.py line 278 at r1 (raw file):

            output.update(component.to_chunk(q))
        # remove all empty components, this is the "empty" field
        output.pop("empty")

same here an extra loop

---

pyorerun/xp_components/marker_trajectories.py line 46 at r1 (raw file):

            else:
                list_frames_to_keep.append(list(range(i - self.nb_frames + 1, i + 1)))
        return list_frames_to_keep

This should be a method of the "abstract" method PersistentComponent. But as a entry it should be frame_id. It will return the frame_idx to show.

Code quote:

    def list_frames_to_keep(self, nb_frames_in_trial: int) -> list[list[int]]:
        """
        For each frame in the trial, it returns a list of the frame numbers that must be displayed.
        Example: A trial composed of 5 frames with a self.nb_frames of 3 frames would get the following output:
        [
            [0],
            [0, 1],
            [0, 1, 2],
            [1, 2, 3],
            [2, 3, 4],
        ]
        Parameters
        ----------
        nb_frames_in_trial: int
            The number of trames that the trial contains
        """
        # Deal with the case where nb_frames is None
        if self.nb_frames is None:
            self.nb_frames = nb_frames_in_trial

        list_frames_to_keep = []
        for i in range(nb_frames_in_trial):
            if i < self.nb_frames:
                list_frames_to_keep.append(list(range(i + 1)))
            else:
                list_frames_to_keep.append(list(range(i - self.nb_frames + 1, i + 1)))
        return list_frames_to_keep

---

pyorerun/phase_rerun.py line 11 at r1 (raw file):

from .timeless import Gravity, Floor, ForcePlate
from .timeless_components import TimelessRerunPhase
from .xp_components import MarkersXp, TimeSeriesQ, ForceVector, Video, VectorXp, MarkerTrajectories

PersistentMarkers

---

pyorerun/phase_rerun.py line 65 at r1 (raw file):

        muscle_activations_intensity: PyoMuscles | np.ndarray = None,
        display_q: bool = False,
        marker_trajectories: MarkerTrajectories = None,

no extra arg

---

pyorerun/phase_rerun.py line 83 at r1 (raw file):

            Whether to display the generalized coordinates q in charts.
        marker_trajectories: MarkerTrajectories
            The marker trajectories to display

no doc

Code quote:

        marker_trajectories: MarkerTrajectories
            The marker trajectories to display

---

pyorerun/phase_rerun.py line 106 at r1 (raw file):

            self.__add_tracked_markers(model, tracked_markers)
            tracked_markers = tracked_markers.to_numpy()[:3, :, :]
        self.models.add_animated_model(model, q, tracked_markers, muscle_colors, marker_trajectories)

no extra arg

---

pyorerun/phase_rerun.py line 299 at r1 (raw file):

                *self.timeless_components.component_names,
            ]:
                rr.log(component, rr.Clear(recursive=False))

I'm surprised nothing has changed here.

Code quote:

        for name, chunk in self.xp_data.to_chunk().items():
            rr.send_columns(
                name,
                times=times,
                components=chunk,
            )

        for name, chunk in self.models.to_chunk().items():
            rr.send_columns(
                name,
                times=times,
                components=chunk,
            )

        if clear_last_node:
            rr.set_time_seconds("stable_time", self.t_span[-1])
            for component in [
                *self.models.component_names,
                *self.xp_data.component_names,
                *self.timeless_components.component_names,
            ]:
                rr.log(component, rr.Clear(recursive=False))

[EveCharbie]

Reviewable status: 9 of 14 files reviewed, 29 unresolved discussions (waiting on @Ipuch)

---

examples/biorbd/msk_model.py line 30 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

I think I would store this information in model.options.persistent_markers=PersistentMarkers(name=["ULNA"], nb_frames=20)

Done.

---

examples/biorbd/msk_model.py line 34 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

so this new extra argument will be gone.

Done.

---

pyorerun/__init__.py line 31 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

PersistentMarkers

Actually there is already something caller PersistentMarkers :/ We can talk about it.

---

pyorerun/model_phase.py line 6 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

PersistentMarkers

Done.

---

pyorerun/model_phase.py line 38 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

PersistentMarkers

Done.

---

pyorerun/model_phase.py line 46 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

no extra_argument

Done.

---

pyorerun/phase_rerun.py line 11 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

PersistentMarkers

Done.

---

pyorerun/phase_rerun.py line 65 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

no extra arg

Done.

---

pyorerun/phase_rerun.py line 83 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

no doc

Done.

---

pyorerun/phase_rerun.py line 106 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

no extra arg

Done.

---

pyorerun/model_components/model_markers.py line 66 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

missing methods

def to_rerun(self, q: np.ndarray) -> None:
rr.log(
self.name,
self.to_component(q),
)

The "to_component" should be re-written to stack all the frames in q.shape[1]
def to_component(self, q: np.ndarray) -> rr.Points3D:
return rr.Points3D(
positions=self.callable_markers(q),
radii=self.marker_properties.radius_to_rerun(),
colors=self.marker_properties.color_to_rerun(),
labels=self.marker_properties.markers_names,
show_labels=self.marker_properties.show_labels_to_rerun(),
)

To be discussed in voice call: I don't understand how to do the to_component, since in rerun_by_frame we only have access to the one frame of Q.

---

pyorerun/model_components/model_markers.py line 85 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

I think this should be somehow displaced to the model to get a callable restrained to the considered markers.

At this point, I do not have access to the model :/

---

pyorerun/model_components/model_updapter.py line 26 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

delete too

Done.

---

pyorerun/model_components/model_updapter.py line 222 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

by using partial, you can partially "feed" some inputs without having to execute it.

callable_markers=partial(self.model.markers, idx=marker_idx)

Thus, it would require to update the interface of models, biorbd and opensim i guess

Need to discuss

---

pyorerun/model_components/model_updapter.py line 244 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

I think this is safer, to create a new methods for persistent_components()

Done.

---

pyorerun/model_components/model_updapter.py line 264 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

We need to add an extra loop

for pcomponent in self.persistent_components:
pcomponent.to_rerun(q:np.dnarray)

Done.

---

pyorerun/model_components/model_updapter.py line 278 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

same here an extra loop

Done.

---

pyorerun/model_interfaces/abstract_model_interface.py line 92 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

I think the model, should be called only in a one-frame manner. This method should disappear

to be discussed

---

pyorerun/model_interfaces/biorbd_model_interface.py line 131 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

This method should disappear

to be discussed

---

pyorerun/model_interfaces/osim_model_interface.py line 239 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

This method should disappear

to be discussed

[Ipuch]

@Ipuch reviewed 1 of 14 files at r1, 11 of 11 files at r2, all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @EveCharbie)

---

pyorerun/phase_rerun.py line 299 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

I'm surprised nothing has changed here.

We need to think by frame first.

---

pyorerun/abstract/abstract_class.py line 32 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

frame_range

but not sure, if it is the right behavior we should have yet.

keep "frame_range"

---

pyorerun/model_components/model_markers.py line 66 at r1 (raw file):

Previously, EveCharbie (Eve Charbonneau) wrote…

To be discussed in voice call: I don't understand how to do the to_component, since in rerun_by_frame we only have access to the one frame of Q.

let as it is

---

pyorerun/model_components/model_markers.py line 79 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

This should not exist in the class

solved with the partial function

---

pyorerun/model_components/model_updapter.py line 222 at r1 (raw file):

Previously, EveCharbie (Eve Charbonneau) wrote…

Need to discuss

self.model.markers(q=q,idx=idx)
new_callable = partial(self.model, idx=[1,5])
new_callable(q)

---

pyorerun/model_interfaces/abstract_model_interface.py line 92 at r1 (raw file):

Previously, EveCharbie (Eve Charbonneau) wrote…

to be discussed

to remove

---

pyorerun/model_interfaces/biorbd_model_interface.py line 131 at r1 (raw file):

Previously, EveCharbie (Eve Charbonneau) wrote…

to be discussed

discussed

---

pyorerun/model_interfaces/osim_model_interface.py line 239 at r1 (raw file):

Previously, EveCharbie (Eve Charbonneau) wrote…

to be discussed

discussed

---

pyorerun/xp_components/persistent_marker_options.py line 6 at r2 (raw file):

class PersistentMarkerOptions(PersistentComponent):

remove inheritage