[JOSS Review] Minor issues

[zexinyang]

Hi @gattia and @siavashk,

This is part of the JOSS review process. I found two minor issues after testing your PyCPD package. Could you please address them?

• Found an error when running python examples/fish_deformable_3D_register_with_subset_of_points.py:

  Traceback (most recent call last):
  File "examples/fish_deformable_3D_register_with_subset_of_points.py", line 55, in <module>
  main()
  File "examples/fish_deformable_3D_register_with_subset_of_points.py", line 43, in main
  YT = reg.transform_point_cloud(Y=Y)
  File "/home/geo3d/miniconda3/envs/joss-pycpd/lib/python3.8/site-packages/pycpd/deformable_registration.py", line 68, in transform_point_cloud
  return Y + np.dot(self.G, self.W)
  ValueError: operands could not be broadcast together with shapes (182,3) (91,3)

  This error also failed the pytest:

  
  ============================= test session starts ==============================
  platform linux -- Python 3.8.13, pytest-7.1.2, pluggy-1.0.0
  rootdir: /home/geo3d/Zexin/project/pycpd
  collected 7 items                                                              

testing/affine_test.py .. [ 28%] testing/deformable_test.py ..F [ 71%] testing/rigid_test.py .. [100%]

=================================== FAILURES =================================== ___ test_3D_low_rank ___

def test_3D_low_rank():
    fish_target = np.loadtxt('data/fish_target.txt')
    X1 = np.zeros((fish_target.shape[0], fish_target.shape[1] + 1))
    X1[:, :-1] = fish_target
    X2 = np.ones((fish_target.shape[0], fish_target.shape[1] + 1))
    X2[:, :-1] = fish_target
    X = np.vstack((X1, X2))

    fish_source = np.loadtxt('data/fish_source.txt')
    Y1 = np.zeros((fish_source.shape[0], fish_source.shape[1] + 1))
    Y1[:, :-1] = fish_source
    Y2 = np.ones((fish_source.shape[0], fish_source.shape[1] + 1))
    Y2[:, :-1] = fish_source
    Y = np.vstack((Y1, Y2))

    reg = DeformableRegistration(**{'X': X, 'Y': Y, 'low_rank': True})
    TY, _ = reg.register()
    assert_array_almost_equal(TY, X, decimal=0)

    rand_pts = np.random.randint(Y.shape[0], size=int(Y.shape[0]/2))

  TY2 = reg.transform_point_cloud(Y=Y[rand_pts, :])

testing/deformable_test.py:56:

---

self = <pycpd.deformable_registration.DeformableRegistration object at 0x7f5ff28b62b0> Y = array([[-0.28181282, -0.79432332, 1. ], [-0.37639549, -0.58040108, 1. ], [-0.91641226, ...1006222, 1. ], [-0.10490431, -0.60374743, 1. ], [-0.50945755, 0.00261754, 1. ]])

def transform_point_cloud(self, Y=None):
    """
    Update a point cloud using the new estimate of the deformable transformation.

    """
    if Y is None:
        self.TY = self.Y + np.dot(self.G, self.W)
        return
    else:

      return Y + np.dot(self.G, self.W)

E ValueError: operands could not be broadcast together with shapes (91,3) (182,3)

../../../miniconda3/envs/joss-pycpd/lib/python3.8/site-packages/pycpd/deformable_registration.py:68: ValueError =========================== short test summary info ============================ FAILED testing/deformable_test.py::test_3D_low_rank - ValueError: operands co... ========================= 1 failed, 6 passed in 0.43s ==========================

- A few warnings can be fixed by replacing `is not` with `!=`:

/home/geo3d/Zexin/project/pycpd/pycpd/rigid_registration.py:45: SyntaxWarning: "is not" with a literal. Did you mean "!="? if R is not None and (R.ndim is not 2 or R.shape[0] is not self.D or R.shape[1] is not self.D or not is_positive_semi_definite(R)): /home/geo3d/Zexin/project/pycpd/pycpd/rigid_registration.py:49: SyntaxWarning: "is not" with a literal. Did you mean "!="? if t is not None and (t.ndim is not 2 or t.shape[0] is not 1 or t.shape[1] is not self.D): /home/geo3d/Zexin/project/pycpd/pycpd/rigid_registration.py:49: SyntaxWarning: "is not" with a literal. Did you mean "!="? if t is not None and (t.ndim is not 2 or t.shape[0] is not 1 or t.shape[1] is not self.D): /home/geo3d/Zexin/project/pycpd/pycpd/affine_registration.py:30: SyntaxWarning: "is not" with a literal. Did you mean "!="? if B is not None and (B.ndim is not 2 or B.shape[0] is not self.D or B.shape[1] is not self.D or not is_positive_semi_definite(B)): /home/geo3d/Zexin/project/pycpd/pycpd/affine_registration.py:34: SyntaxWarning: "is not" with a literal. Did you mean "!="? if t is not None and (t.ndim is not 2 or t.shape[0] is not 1 or t.shape[1] is not self.D): /home/geo3d/Zexin/project/pycpd/pycpd/affine_registration.py:34: SyntaxWarning: "is not" with a literal. Did you mean "!="? if t is not None and (t.ndim is not 2 or t.shape[0] is not 1 or t.shape[1] is not self.D):

[gattia]

@zexinyang thanks so much for the review, and for looking into this! I have mostly completed all components of the review and should be submitting my response shortly.

For the errors here, I tried to re-create the errors and couldn't. In the end, it looks like you might have an old version of pycpd installed because the lines of code that have an error don't match the current code. Specifically, there should be an additional if/else statement in the def transform_point_cloud function that tests if you are trying to transform a low-rank version or not.

Could you try re-pulling, installing, and then running the examples/tests?

# activate your environment
cd /home/geo3d/Zexin/project/pycpd
git pull origin master
pip install .
make dev                        # optionally install pytest using make dev if you haven't already. 
make test                        # run the tests. 
python examples/fish_deformable_3D_register_with_subset_of_points.py

[zexinyang]

@gattia Thank you for addressing the issue. Now it works perfectly ;)