XuyangBai / PPF-FoldNet

PyTorch reimplementation for "PPF-FoldNet: Unsupervised Learning of Rotation Invariant 3D Local Descriptors" https://arxiv.org/abs/1808.10322
99 stars 19 forks source link

Make fragment fusion script compliant with multiple Open3D versions. #3

Closed SergioRAgostinho closed 4 years ago

SergioRAgostinho commented 4 years ago

The current PR addresses API changes that occurred in Open3D. Three important points to mention here:

XuyangBai commented 4 years ago

Hi, thank you for your feedback. I didn't test the open3d with version later than 0.7 but line 22 should be

if LooseVersion(o3d.__version__) > LooseVersion("0.7.0.0"):

or the version 0.7.0.0 will be in the wrong branch

SergioRAgostinho commented 4 years ago

I am not witnessing that behaviour.

>>> import open3d as o3d
>>> from distutils.version import LooseVersion
>>> o3d.__version__
'0.9.0.0'
>>> LooseVersion(o3d.__version__) >= LooseVersion("0.9")
True
>>> LooseVersion(o3d.__version__) >= LooseVersion("0.9.0.1")
False
>>> LooseVersion(o3d.__version__) >= LooseVersion("0.10")
False
>>> LooseVersion(o3d.__version__) >= LooseVersion("0.9.1.0")
False
>>> LooseVersion(o3d.__version__) >= LooseVersion("0.9.0.0")
True
>>> LooseVersion(o3d.__version__) >= LooseVersion("0.7.0.0")
True
>>> LooseVersion(o3d.__version__) >= LooseVersion("0.7")
True

What's your distutils version? Mine is 3.6.10.

Interesting. The inequality sign is key to the problem, you can see below.

>>> LooseVersion(o3d.__version__) > LooseVersion("0.9")
True    # It should be false, but subject to interpretation.
>>> LooseVersion(o3d.__version__) > LooseVersion("0.9.0.1")
False
>>> LooseVersion(o3d.__version__) > LooseVersion("0.10")
False
>>> LooseVersion(o3d.__version__) > LooseVersion("0.9.1.0")
False
>>> LooseVersion(o3d.__version__) > LooseVersion("0.9.0.0")
False 
>>> LooseVersion(o3d.__version__) > LooseVersion("0.7.0.0")
True
>>> LooseVersion(o3d.__version__) > LooseVersion("0.7")
True

However in the patch I submitted I'm using >= and not >. Thus things are falling under the right branch. Do you still want me to modify the version to the complete semver string?

XuyangBai commented 4 years ago

@SergioRAgostinho There might be something wrong with distutil. In my environment, things go wrong if using 0.7.0.0 version. My >= and > return different values. 截屏2020-02-27下午5 34 21

Maybe just leave it as so, I use open3d 0.7.0.0 for my whole project and it takes time to make it all compliant with other versions. Anyway, thank you so much !

SergioRAgostinho commented 4 years ago

Maybe just leave it as so, I use open3d 0.7.0.0 for my whole project and it takes time to make it all compliant with other versions. Anyway, thank you so much !

No problem.