added cffd and variations #268

guglielmopadula · 2023-09-11T09:40:41Z

Hi, these are the modifications of the library used in the preprint https://arxiv.org/abs/2308.03662.

mtezzele

Thank you for the contribution. Please add a link to the tutorial with a description in the readme you can find in the tutorials folder.
Please add the tests.

mtezzele · 2023-09-11T13:15:24Z

pygem/cffd.py

+        saved_parameters=self._save_parameters()
+        A,b=self._compute_linear_map(src_pts,saved_parameters.copy())
+        d=A@saved_parameters[self.indices]+b
+        deltax=np.linalg.inv(self.M)@A.T@np.linalg.inv((A@np.linalg.inv(self.M)@A.T))@(self.valconstraint-d)


Please add a space between the operators

mtezzele · 2023-09-11T13:15:41Z

pygem/cffd.py

+            :rtype: numpy.ndarray
+        '''
+        tmp=np.zeros([*self.n_control_points,3])
+        tmp[:,:,:,0]=self.array_mu_x


Space after commas

mtezzele · 2023-09-11T13:15:59Z

pygem/cffd.py

+        :param np.ndarray saved_parameters
+        :return a tuple containing the coefficient and the intercept
+        :rtype tuple(np.ndarray,np.ndarray)
+


Remove unnecessary empty lines in the doc

mtezzele · 2023-09-11T13:16:30Z

pygem/cffd.py

+        self._load_parameters(saved_parameters)
+        def_pts=super().__call__(src_pts)
+        outputs[0]=self.linconstraint(def_pts)
+        for i in range(1,n_indices+1):


Add some inline comments please

mtezzele · 2023-09-11T13:16:55Z

pygem/vffd.py

+"""
+Utilities for performing Volume Free Form Deformation (BFFD).
+
+:Theoretical Insight:


I don't think this will be rendered as intended in the online doc

mtezzele · 2023-09-11T13:17:29Z

pygem/bffd.py

+class BFFD(CFFD):
+    '''
+    Class that handles the Barycenter Free Form Deformation on the mesh points.
+


Arguments doc is missing

Hi. For this class I did not add any argument doc because they are exactly the same as the mother class. The only difference is that in the child class a mother class member has a default value. I should add them anyway?
Thanks for your comments.

Yes please, so the online documentation will be self-contained.

mtezzele · 2023-09-11T13:17:49Z

pygem/cffd.py

+    Class that handles the Constrained Free Form Deformation on the mesh points.
+
+    :cvar callable linconstraint: it defines the F of the constraint F(x)=c.
+


remove empty lines

mtezzele · 2023-09-11T13:18:01Z

pygem/cffd.py

+
+    def __call__(self, src_pts):
+        saved_parameters=self._save_parameters()
+        A,b=self._compute_linear_map(src_pts,saved_parameters.copy())


…n with comments

mtezzele · 2023-09-12T16:16:43Z

tutorials/README.md

@@ -11,7 +11,7 @@ In this folder we collect several useful tutorials in order to understand the pr
 | Tutorial4&#160;[[.ipynb](/~https://github.com/mathLab/PyGeM/blob/master/tutorials/tutorial4/tutorial-4-idw.ipynb),&#160;[.py](/~https://github.com/mathLab/PyGeM/blob/master/tutorials/tutorial4/tutorial-4-idw.py),&#160;[.html](http://mathlab.github.io/PyGeM/tutorial-4-idw.html)] | inverse distance weighting to deform a cubic mesh | `pygem.IDW`  | `numpy.ndarray` |
 | Tutorial5&#160;[[.ipynb](/~https://github.com/mathLab/PyGeM/blob/master/tutorials/tutorial5/tutorial-5-file.ipynb),&#160;[.py](/~https://github.com/mathLab/PyGeM/blob/master/tutorials/tutorial5/tutorial-5-file.py),&#160;[.html](http://mathlab.github.io/PyGeM/tutorial-5-file.html)] | free-form deformation to deform an object contained to file | `pygem.FFD`  | `.vtp` file, `.stl` file |
 | Tutorial6&#160;[[.ipynb](/~https://github.com/fAndreuzzi/PyGeM/blob/master/tutorials/tutorial6/tutorial-6-ffd-rbf.ipynb),&#160;[.py](/~https://github.com/fAndreuzzi/PyGeM/blob/master/tutorials/tutorial6/tutorial-6-ffd-rbf.py),&#160;[.html](http://mathlab.github.io/PyGeM/tutorial-6-ffd-rbf.html)] | interpolation of an OpenFOAM mesh after a deformation | `pygem.FFD/RBF`  | OpenFOAM |
-
+| Tutorial8&#160;[[.ipynb](/~https://github.com/guglielmopadula/PyGeM/blob/master/tutorials/tutorial8/tutorial-8-cffd.ipynb),&#160;[.py](/~https://github.com/guglielmopadula/PyGeM/blob/master/tutorials/tutorial8/tutorial-8-cffd.py),&#160;[.html](http://mathlab.github.io/PyGeM/tutorial-8-cffd.html)] | constrained free form deformation | `pygem.CFFD/BFFD/VFFD`  | `.stl` file |


Do not use the url of your fork. Use the mathlab one instead, please:/~https://github.com/mathLab/PyGeM/...
It should be tutorial7 and not 8.

May I ask you to fix also the path of tutorial 6 (I know it's not your fault but I spotted it only now...).

Thanks!

ndem0 · 2023-09-15T07:34:20Z

pygem/cffd.py

+        saved_parameters=self._save_parameters()
+        A, b = self._compute_linear_map(src_pts, saved_parameters.copy())
+        d = A @ saved_parameters[self.indices] + b
+        deltax = np.linalg.inv(self.M) @ A.T @ np.linalg.inv((A @ np.linalg.inv(self.M)@ A.T)) @ (self.valconstraint - d)


You're computing the inverse of M twice in same line. Please, save intermediate result in a variable

ndem0 · 2023-09-15T07:54:37Z

Dear @guglielmopadula, thanks for the additions! The contribution is very nice, but there are some gray point in the implementation. I started to left some comments, if you can please use also the code_formatter.sh before pushing in order to respect the code style (PEP8).

ndem0 · 2023-09-15T07:34:38Z

pygem/cffd.py

+        saved_parameters=self._save_parameters()
+        A, b = self._compute_linear_map(src_pts, saved_parameters.copy())
+        d = A @ saved_parameters[self.indices] + b
+        deltax = np.linalg.inv(self.M) @ A.T @ np.linalg.inv((A @ np.linalg.inv(self.M)@ A.T)) @ (self.valconstraint - d)


use https://numpy.org/devdocs/reference/generated/numpy.linalg.multi_dot.html

ndem0 · 2023-09-15T07:35:03Z

pygem/cffd.py

+        self._load_parameters(saved_parameters)
+        return self.ffd(src_pts)
+
+    def ffd(self,src_pts):


blank space after comma

ndem0 · 2023-09-15T07:35:48Z

pygem/cffd.py

+        saved_parameters_bak=saved_parameters.copy() #saving ffd parameters
+        n_indices=len(self.indices)
+        inputs=np.zeros([n_indices+1,n_indices+1])
+        outputs=np.zeros([n_indices+1,self.valconstraint.shape[0]])
+        np.random.seed(0)
+        for i in range(n_indices+1): ##now we generate the interpolation points
+            tmp=np.random.rand(1,n_indices)
+            tmp=tmp.reshape(1,-1)
+            inputs[i]=np.hstack([tmp, np.ones((tmp.shape[0], 1))]) #dependent variable
+            saved_parameters[self.indices]=tmp 
+            self._load_parameters(saved_parameters) #loading the depent variable as a control point
+            def_pts=super().__call__(src_pts) #computing the deformation with the dependent variable
+            outputs[i]=self.linconstraint(def_pts) #computing the independent variable
+        sol=np.linalg.lstsq(inputs,outputs,rcond=None) #computation of the linear map
+        A=sol[0].T[:,:-1] #coefficient
+        b=sol[0].T[:,-1] #intercept
+        self._load_parameters(saved_parameters_bak) #restoring the original FFD parameters
+        return A,b


Please change using PEP8 style

ndem0 · 2023-09-15T07:36:51Z

pygem/cffd.py

+
+
+# I see that a similar function already exists in pygem.utils, but it does not work for inputs and outputs of different dimensions
+    def _compute_linear_map(self,src_pts,saved_parameters):


Does it do same of the function you're mentioning? Why do we should keep the older version?

ndem0 · 2023-09-15T07:37:33Z

pygem/vffd.py

+    def __call__(self,src_pts):
+        self.vweight=np.abs(self.vweight)/np.sum(np.abs(self.vweight))
+        indices_bak=deepcopy(self.indices)
+        self.indices=np.array(self.indices)
+        indices_x=self.indices[self.indices%3==0].tolist()
+        indices_y=self.indices[self.indices%3==1].tolist()
+        indices_z=self.indices[self.indices%3==2].tolist()
+        indexes=[indices_x,indices_y,indices_z]
+        diffvolume=self.valconstraint-self.linconstraint(self.ffd(src_pts))
+        for i in range(3):
+            self.indices=indexes[i]
+            self.M=np.eye(len(self.indices))
+            self.valconstraint=self.linconstraint(self.ffd(src_pts))+self.vweight[i]*(diffvolume)
+            _=super().__call__(src_pts)
+        tmp=super().__call__(src_pts)
+        self.indices=indices_bak
+        return tmp


ndem0 · 2023-09-15T07:37:58Z

pygem/vffd.py

+            self.indices=indexes[i]
+            self.M=np.eye(len(self.indices))
+            self.valconstraint=self.linconstraint(self.ffd(src_pts))+self.vweight[i]*(diffvolume)
+            _=super().__call__(src_pts)


What's the meaning of this line?

ndem0 · 2023-09-15T07:43:10Z

pygem/cffd.py

+        self._load_parameters(saved_parameters)
+        return self.ffd(src_pts)
+
+    def ffd(self,src_pts):


What's the meaning of this method?

ndem0 · 2023-09-15T07:46:37Z

pygem/vffd.py

+        self.vweight=np.abs(self.vweight)/np.sum(np.abs(self.vweight))
+        indices_bak=deepcopy(self.indices)
+        self.indices=np.array(self.indices)
+        indices_x=self.indices[self.indices%3==0].tolist()


very inefficient! What kind of reshaping are you trying to do?

ndem0 · 2023-09-15T07:49:26Z

pygem/bffd.py

+if __name__ == "__main__":
+        from pygem import BFFD
+        import numpy as np
+        bffd = BFFD()
+        bffd.read_parameters('tests/test_datasets/parameters_test_ffd_sphere.prm')
+        original_mesh_points = np.load('tests/test_datasets/meshpoints_sphere_orig.npy')
+        b=bffd.linconstraint(original_mesh_points)
+        bffd.valconstraint=b
+        bffd.indices=np.arange(np.prod(bffd.n_control_points)*3).tolist()
+        bffd.M=np.eye(len(bffd.indices))
+        new_mesh_points = bffd(original_mesh_points)
+        assert np.isclose(np.linalg.norm(bffd.linconstraint(new_mesh_points)-b),np.array([0.]))


Please remove

ndem0 · 2023-09-15T12:17:33Z

pygem/cffd.py

+        >>> assert np.isclose(np.linalg.norm(fun(new_mesh_points)-b),np.array([0.]))
+    """
+
+    def __init__(self, n_control_points=None):


(self, n_control_points=None, linear_constrain=None, val, indices=None, weight_matrix=None):

ndem0 · 2023-10-16T12:11:28Z

Dear @guglielmopadula, I just quickly looked at the new code and it's exactly what I meant! Perfect, thank you! Only one change please: the class VFFD and BFFD are now in the same file (cffd.py). It is not an error but I would consider to keep them in a separate files (no matter their length) in order to be more accessible by new users

ndem0 · 2023-10-30T17:09:08Z

Dear @guglielmopadula, now some of the tests are failing. Could you check? In your computer, is everything fine?

guglielmopadula · 2023-10-30T17:26:50Z

Yes, I and I see that in the ubuntu 3.7 version they don't fail, so maybe there are some problems with the virtual machines?

added cffd and variations

43bd627

mtezzele reviewed Sep 11, 2023

View reviewed changes

guglielmopadula added 2 commits September 12, 2023 10:22

reformat code, fixed sphinx, add test, simplied interpolation functio…

453dcc3

…n with comments

trying to fix links

5d3c2c1

mtezzele reviewed Sep 12, 2023

View reviewed changes

added tutorial py file, fix tutorials readme and tutorial number

1b0f8f2

ndem0 reviewed Sep 15, 2023

View reviewed changes

guglielmopadula added 11 commits September 16, 2023 11:00

started refactoring and deletion of two useless lines

de60a57

completed transition from indexes to maskes

e501eae

moved to multi_dot in main function

7278e2e

moved to second multi_dot and deleted useless print

d86dd07

one more test and general simplication

5f625b2

refactored weight matrix

6cd01a1

deleted call function to CFFD to highlight the working principle

2a79e6b

fixed init and started working on docs

481a6e5

fixing other init files

dcae4e9

finished fixing init and docs

5d3dfee

fixed tutorials, use of code formatter

1f1a150

ndem0 reviewed Sep 18, 2023

View reviewed changes

guglielmopadula added 8 commits September 18, 2023 14:17

text more clearer

70e9299

add description to vffd

235e251

applied pep8 to py and ipynb

0f03419

Started merging: test passes

c69b7cd

outer and inner function merged: test passing

bdf74cb

started working on hyperparameter optimization. Weight matrix is back.

92cc306

added some new parameter and stl files

519512e

fixed tutorials and documentation, applied code formatter

7ae6bf5

separate things again, fixed docs and tutorialx

4a9d160

ndem0 merged commit 3c11eb1 into mathLab:master Nov 9, 2023
1 of 6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added cffd and variations #268

added cffd and variations #268

guglielmopadula commented Sep 11, 2023

mtezzele left a comment

mtezzele Sep 11, 2023

mtezzele Sep 11, 2023

mtezzele Sep 11, 2023

mtezzele Sep 11, 2023

mtezzele Sep 11, 2023

mtezzele Sep 11, 2023

guglielmopadula Sep 11, 2023

mtezzele Sep 11, 2023 •

edited

Loading

mtezzele Sep 11, 2023

mtezzele Sep 11, 2023

mtezzele Sep 12, 2023

ndem0 Sep 15, 2023

ndem0 commented Sep 15, 2023

ndem0 Sep 15, 2023

ndem0 Sep 15, 2023

ndem0 Sep 15, 2023

ndem0 Sep 15, 2023

ndem0 Sep 15, 2023

ndem0 Sep 15, 2023

ndem0 Sep 15, 2023

ndem0 Sep 15, 2023

ndem0 Sep 15, 2023

ndem0 Sep 15, 2023

ndem0 commented Oct 16, 2023

ndem0 commented Oct 30, 2023

guglielmopadula commented Oct 30, 2023

		Class that handles the Constrained Free Form Deformation on the mesh points.

		:cvar callable linconstraint: it defines the F of the constraint F(x)=c.



		# I see that a similar function already exists in pygem.utils, but it does not work for inputs and outputs of different dimensions
		def _compute_linear_map(self,src_pts,saved_parameters):

added cffd and variations #268

added cffd and variations #268

Conversation

guglielmopadula commented Sep 11, 2023

mtezzele left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtezzele Sep 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ndem0 commented Sep 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ndem0 commented Oct 16, 2023

ndem0 commented Oct 30, 2023

guglielmopadula commented Oct 30, 2023

mtezzele Sep 11, 2023 •

edited

Loading