Ipuch / bioNC

Natural Coordinates with python for biomechanics
MIT License
11 stars 4 forks source link

[WIP] Add HMP constraints to InverseKinematics #98

Closed aaiaueil closed 1 year ago

aaiaueil commented 1 year ago

To follow the different idea.

This change is Reviewable

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 230 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
self._markers_sym = MX.sym("markers", (3, self.nb_markers)) self._camera__parameters_sym = MX. self._gaussian_parameters_sym = MX. example: def addition(a,b,c): return a+b

we should verify here that the symbolic variables I created have the appropriate shape depending on what they do afterwards

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 195 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
Use self.experimental_markers is None as boolean.

Wouldn't it be better to use self.experimental_heatmaps is not None as a boolean to indicate we use experimental heatmaps? Might be easier to understand

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 520 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
camera_mx, gaussian_mx as entry.

Putting it here so we can discuss it later : as my prpblem is under-constrained, some markers are used twice to build the model. For example, 'right_knee' is used to build segment right thigh and segment right shank. I don't want right_knee to weight twice in the optimization, how should I do? Should I build the model by doing model["RSHANK"].add_marker(right_knee) for the shank only and not for the right hip, or should I add said marker for both segments and add conditions in the optimization?

Ipuch commented 1 year ago

Nothing was pushed, Is that normal ? I would expect at least one commit to be pushed, after my review to turn the PR into RTR.

aaiaueil commented 1 year ago

nope, not normal! doing it right now

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 158 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
We need a discussion on the format of your data to simplify as much as possible the code. This is clearly not acceptable to add string as entry. You have camera parameters and Gaussian parameters for each marker for each frame experimental_heat_map_parameters: ```python dict( camera_parameters= 3 x 4 x n_camera, gaussian_parameters= 5 x n_marker x n_frame x n_camera, ) ```

Modification made in the code calling the IK class. Added experimental_heatmaps as a dictionary in the IK class initialization

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 6 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
ezc3d should not be imported.

Done.

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 15 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
You've blacked the wrong way. black . - l120 Please restore all the line ...

Done.

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 18 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
unblack.

Done.

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 46 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
unblack

Done.

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 194 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
Unblack

Done.

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 231 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
change the boolean

Done.

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 233 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
self.experimental_heat_map_paremeters["gaussian_parameters"].shape[2]

Done.

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 277 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
[self._Q_sym, self._markers_sym, self._camera_parameter_sym, self._gaussian_parameter_sym],

Done.

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 397 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
objective = self._objective_function( self._Q_sym, self.experimental_markers[:, :, f] if experimental is None [], self.camera_parameters[:,:,:] if experimental_hmp is None [], self.gaussian_parameters[:,:,f,:] if experimental_hmp is None [], ) def addition(a,b,c,d): return a_sym + 1 + 2 + 3

Alex and Anais talking there : Should we mofify also the natural_marker.constraint (l.139) to be able to adapt to different input value ? We can discuss it tomorrow.

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 672 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
unblack

Done.

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 256 at r3 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
if experimental_markers is not None self.nb_frames = else: self.nb_frames = if self.nb_markers = else: self.nb_markers = if experimental_markers is not None self.nb_camera = 0 else: self.nb_frames = XX Once all attributes are filled Create the corresponding mx. And find a way to concatenate dimensions of mx symbolic into a 2d matrix.

I would like to double check with you I didn't do something too random

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 259 at r3 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
self.nb_camera = ...

Done.

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 413 at r3 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
reshape camera_parameters to fit the shape of the mx size

wouldn't it be more consistent if camera_parameters.shape was nb_cameras x 3 x4 instead of 3 x 4 x nb_cameras, especially for reshaping afterwards? Reshaping in 2D would give something like nb_cameras x 12 and would be easier to understand/visualize in my opinion

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 413 at r3 (raw file):

Previously, aaiaueil wrote…
wouldn't it be more consistent if camera_parameters.shape was nb_cameras x 3 x4 instead of 3 x 4 x nb_cameras, especially for reshaping afterwards? Reshaping in 2D would give something like nb_cameras x 12 and would be easier to understand/visualize in my opinion

for now I'm leaving it as it is

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 132 at r3 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…
Remove this.

I'm still not sure how I can do without it...

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 132 at r3 (raw file):

Previously, aaiaueil wrote…
I'm still not sure how I can do without it...

or should I code it myself directly in biomechanical_model class?

aaiaueil commented 1 year ago

bionc/bionc_numpy/inverse_kinematics.py line 132 at r3 (raw file):

Previously, aaiaueil wrote…
or should I code it myself directly in biomechanical_model class?

update : did it but can't figure out how to get the values (:

Ipuch commented 1 year ago

Don't merge the main branch until you finish your dev.

Ipuch commented 1 year ago

I want to close that pull request, it has been a while now. Expected work before next meeting:

codeclimate[bot] commented 1 year ago

Code Climate has analyzed commit 0dffe6f1 and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 2
Duplication 2

View more on Code Climate.