TRI-ML / dgp

ML Dataset Governance Policy for Autonomous Vehicle Datasets
https://tri-ml.github.io/dgp/
MIT License
93 stars 63 forks source link

style: modify parallel domain metadata with `make build-proto` #102

Closed nehalmamgain closed 2 years ago

nehalmamgain commented 2 years ago

I made a virtual environment following pip install -r requirements-dev.txt (Also tested this in the virtual environment following this.)

Although not documented in CONTRIBUTING (fixed now with https://github.com/TRI-ML/dgp/pull/103), one of the former DGP maintainers asked to run make build-proto on this PR.

When running make build-proto, I saw a previous commit I made be formatted differently. This PR commits the affected file with the output of make build-proto so that other users don't see a file unrelated to their PR also show up in their git status when contributing to this repository.


This change is Reviewable

nehalmamgain commented 2 years ago

+reviewer:@tk-woven +reviewer:@kuanleetri

nehalmamgain commented 2 years ago

I edited the commit message to follow linter expectations

nehalmamgain commented 2 years ago

The generated file was built from the source tree as-is, no local changes?

Yup, exactly!

We can probably extend DGP CI to build the protos and make sure everything us up to date as one of the checks.

That sounds cool!

The CI failed. I don't believe the test is related to PD metadata though. Temporary failure (since it actually passed before; twice!)? @tk-woven Sorry to bother you, but I don't have access to re-run this workflow without pushing a change. Could you please re-run it?

=================================== FAILURES ===================================
______________________ TestTorchUtilities.test_pose_utils ______________________

self = <tests.test_torch_extension_utils.TestTorchUtilities testMethod=test_pose_utils>

    def test_pose_utils(self):
        """Test pose class in dgp.utils.torch_extension.Pose and dgp.utils.torch_extension.QuaternionPose"""

        # Test pose transforms
        npposes = [NumpyPose(wxyz=pvec[:4], tvec=pvec[4:]) for pvec in self.pvecs]
        poses = [Pose(tf) for tf in self.tfs]
        qposes = [QuaternionPose.from_matrix(tf) for tf in self.tfs]

        # Test matrix composition of transformations
        final_pose_np = functools.reduce(lambda x, y: x @ y, [tf.numpy() for tf in self.tfs])
        final_pose_torch = functools.reduce(lambda x, y: x @ y, self.tfs)
        assert_true(np.allclose(final_pose_np, final_pose_torch.numpy(), atol=1e-6))

        # Test Pose manifold composition of transformations
        final_pose_NumpyPose = functools.reduce(lambda x, y: x * y, npposes)
        final_pose_Pose = functools.reduce(lambda x, y: x * y, poses)
        final_pose_QuaternionPose = functools.reduce(lambda x, y: x * y, qposes)
        assert_true(np.allclose(final_pose_np, final_pose_NumpyPose.matrix, atol=1e-6))
        assert_true(np.allclose(final_pose_np, final_pose_Pose.matrix.numpy(), atol=1e-6))
        assert_true(np.allclose(final_pose_np, final_pose_QuaternionPose.matrix.numpy(), atol=1e-6))

        def make_random_points(B=1, N=100):
            return torch.from_numpy(np.random.rand(B, 3, N)).type(torch.float)

        # Test single point cloud transformations for some implementations
        X = make_random_points()
        Xt_ = X[0].numpy()
        X_ = Xt_.T

        # Test point cloud transformations
        X1 = final_pose_Pose * X
        X2 = final_pose_QuaternionPose * X
        X3 = final_pose_NumpyPose * X_
        X4 = final_pose_np.dot(np.vstack([Xt_, np.ones((1, len(X_)))]))

>       assert_true(np.allclose(X1.numpy(), X2.numpy(), atol=1e-6))
E       AssertionError: False is not true

tests/test_torch_extension_utils.py:98: AssertionError
=========================== short test summary info ============================
FAILED tests/test_torch_extension_utils.py::TestTorchUtilities::test_pose_utils
=================== 1 failed, [57](https://github.com/TRI-ML/dgp/runs/7604952531?check_suite_focus=true#step:7:58) passed, 1 skipped in 41.[76](https://github.com/TRI-ML/dgp/runs/7604952531?check_suite_focus=true#step:7:77)s ===================
tk-woven commented 2 years ago

Re-running now!

nehalmamgain commented 2 years ago

Thank you, the coverage test passed. Please re-review :bowing_woman:

nehalmamgain commented 2 years ago

I'll leave it up to you and @kuanleetri to merge this, thanks!

tk-woven commented 2 years ago

Got it! I'm not very familiar with this process yet. @kuanleetri , I'd like your input if you have a moment. If some time passes and I can study this process for myself, I'll merge it on my own :)

nehalmamgain commented 2 years ago

@tk-woven Btw could you please support here and check the merge process/reach out to @kuanleetri ? It's not so important to get this PR merged but it's extremely important to merge https://github.com/TRI-ML/dgp/pull/101 .

nehalmamgain commented 2 years ago

Rebased off of master.