Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.9.0 Release #2399

Closed
14 of 16 tasks
SergioRAgostinho opened this issue Aug 26, 2018 · 74 comments
Closed
14 of 16 tasks

1.9.0 Release #2399

SergioRAgostinho opened this issue Aug 26, 2018 · 74 comments
Milestone

Comments

@SergioRAgostinho
Copy link
Member

SergioRAgostinho commented Aug 26, 2018

Task list

  • Go through all PRs for the milestone
  • Update Changelog
  • Start a release branch
  • (If needed) Revert whatever unintentional API/ABI breakage that might have occurred in the release branch
  • Bump version to x.x.x in CMakeLists.txt
  • Bump version off the release badge in README.md
  • Find replace all occurrences of x.x.x.99 CMake requirements in the tutorials and examples
  • Try to build and test on all relevant platforms
  • Finalize changelog and fix the release date
  • Tag
  • Remove the release branch
  • Add binaries to the GitHub release page.
  • Add link doc link to tutorials page
  • Bump version to x.x.x-dev in CMakeLists.txt
  • Send announcements to the mailing lists
  • Throw a party!
@SergioRAgostinho
Copy link
Member Author

I reduced the milestone task list to the only blocker reported here.

@SergioRAgostinho SergioRAgostinho added this to the pcl-1.9.0 milestone Sep 25, 2018
@SergioRAgostinho
Copy link
Member Author

SergioRAgostinho commented Sep 28, 2018

@PointCloudLibrary/testers @svenevs @claudiofantacci @jasjuang

Big favor: can you build the library at this commit a20e3c2 , run the tests and try to recompile your usual applications with it and report results?

Edit: Just to help

cmake .. -DBUILD_simulation=ON -DBUILD_global_tests=ON -DBUILD_examples=ON -DBUILD_tools=ON -DBUILD_apps=ON -DBUILD_apps_3d_rec_framework=ON -DBUILD_apps_cloud_composer=ON -DBUILD_apps_in_hand_scanner=ON -DBUILD_apps_modeler=ON -DBUILD_apps_optronic_viewer=ON -DBUILD_apps_point_cloud_editor=ON [-DBUILD_CUDA=ON -DBUILD_GPU=ON -DBUILD_cuda_apps=ON -DBUILD_cuda_io=ON -DBUILD_gpu_people=ON -DBUILD_gpu_surface=ON -DBUILD_gpu_tracking=ON]

Ubuntu 18.04, GPU=OFF, CUDA=OFF:
Builds

The following tests FAILED:
         13 - common_eigen (Failed)
         24 - feature_rift_estimation (Failed)
         26 - feature_cppf_estimation (Failed)
         28 - feature_pfh_estimation (Failed)
         51 - filters_sampling (Failed)
         86 - registration_api (Failed)
        109 - sample_consensus_line_models (Failed)

OS X 10.13.6, GPU=OFF, CUDA=OFF:
Builds

The following tests FAILED:
	 13 - common_eigen (Failed)
	 24 - feature_rift_estimation (Failed)
	 26 - feature_cppf_estimation (Failed)
	 28 - feature_pfh_estimation (Failed)
	 51 - filters_sampling (Failed)
	 71 - io_ply_mesh_io (Failed)
	 85 - a_registration_test (Failed)
	107 - sample_consensus_line_models (Failed)

@claudiofantacci
Copy link
Contributor

Trying now.
I can recompile my code asap, however, I can test it on Monday.

@svenevs
Copy link
Contributor

svenevs commented Sep 28, 2018

Is it sufficient to test without VTK? What CUDA version is being targeted? I can test 9 and 10

@SergioRAgostinho
Copy link
Member Author

Is it sufficient to test without VTK? What CUDA version is being targeted? I can test 9 and 10

Yes. Just do whatever you already have, no need to go bananas on installing third parties. Regarding CUDA, we only saw commits for 9. You can try 10 but if it fails I personally wouldn't delay the release because of it.

@svenevs
Copy link
Contributor

svenevs commented Sep 28, 2018

Sounds good, and yes I agree 10 just came out. I am returning from travels this weekend, if you don't hear from me by end of Sunday for you that means I forgot (sorry, I'm at least being honest)

@jasjuang
Copy link
Contributor

I just recompiled PCL on Ubuntu 18.04 with CUDA 10. Tested with my downstream project. No problem with compilation and all my unit tests passes. The release is good to go on my end!

@claudiofantacci
Copy link
Contributor

Compiling PCL on macOS 10.13.6 with your suggested option and setting the following parameters to ON manually

BUILD_apps_3d_rec_framework      ON                                                                                 
BUILD_apps_cloud_composer        ON                                                                                 
BUILD_apps_in_hand_scanner       ON                                                                                 
BUILD_apps_modeler               ON                                                                                 
BUILD_apps_optronic_viewer       ON                                                                                 
BUILD_apps_point_cloud_editor    ON

I get the following error:

/Users/Claudio/GitHub/pcl/apps/point_cloud_editor/src/selectionTransformTool.cpp:198:15: error: cannot take the address of an rvalue of type 'float'
  float *pt = &((cloud_ptr_->getObjectSpacePoint(*it)).data[X]);
              ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/Claudio/GitHub/pcl/apps/point_cloud_editor/src/selectionTransformTool.cpp:204:10: error: cannot take the address of an rvalue of type 'float'
    pt = &((cloud_ptr_->getObjectSpacePoint(*it)).data[X]);
         ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.
make[2]: *** [apps/point_cloud_editor/CMakeFiles/pcl_point_cloud_editor.dir/src/selectionTransformTool.cpp.o] Error 1
make[1]: *** [apps/point_cloud_editor/CMakeFiles/pcl_point_cloud_editor.dir/all] Error 2

Turning all OFF as by default value (at least that I have)

BUILD_apps_3d_rec_framework      OFF                                                                                 
BUILD_apps_cloud_composer        OFF                                                                                 
BUILD_apps_in_hand_scanner       OFF                                                                                 
BUILD_apps_modeler               OFF                                                                                 
BUILD_apps_optronic_viewer       OFF                                                                                 
BUILD_apps_point_cloud_editor    OFF

the compilation finishes just fine (beside some warnings generated by VTK 8.1).

Running ctest I get the following output:

93% tests passed, 8 tests failed out of 110

Total Test time (real) = 194.66 sec

The following tests FAILED:
	 13 - common_eigen (Failed)
	 24 - feature_rift_estimation (Failed)
	 26 - feature_cppf_estimation (Failed)
	 28 - feature_pfh_estimation (Failed)
	 51 - filters_sampling (Failed)
	 71 - io_ply_mesh_io (Failed)
	 85 - a_registration_test (Failed)
	109 - sample_consensus_line_models (Failed)
Errors while running CTest

@claudiofantacci
Copy link
Contributor

I will report other compilation test on macOS during the week end 😄

@SergioRAgostinho
Copy link
Member Author

@claudiofantacci how did you manage to build point_cloud_editor on Mac? Did you compile Qt4 for yourself? Homebrew is just shipping 5.

@claudiofantacci
Copy link
Contributor

claudiofantacci commented Sep 28, 2018

I enabled BUILD_apps and then manually BUILD_apps_point_cloud_editor to ON. It was by default OFF, like the other options, but I tried anyway. I enabled all the apps because some of them compiled, like 3D_rec_framework, so I tried the other as well. Was I wrong?

@SergioRAgostinho
Copy link
Member Author

I'm quite sure it requires Qt4 to build and that one is not shipped by Homebrew anymore. You probably have a "residual" installation on your computer.

@claudiofantacci
Copy link
Contributor

Maybe I have a residual Qt4, for sure not simlinked. But are you saying that the option should not appear at all?

@SergioRAgostinho
Copy link
Member Author

SergioRAgostinho commented Sep 28, 2018

Can you upload the contents of your CMakeCache.txt?

But are you saying that the option should not appear at all?

I had the impression that it required Qt4. Even the CMake summary tells me that.

--   apps
       not building: 
       |_ 3d_rec_framework: OpenNI was not found or was disabled by the user.
       |_ cloud_composer: Cloud composer requires QVTK
       |_ in_hand_scanner: OpenNI was not found or was disabled by the user.
       |_ modeler: VTK was not built with Qt support.
       |_ optronic_viewer: Qt was not found.
       |_ point_cloud_editor: Qt4 was not found.

Edit2: I was wrong... apparently it works with Qt4 or 5.

@claudiofantacci
Copy link
Contributor

Sure I will!
I’m drinking some wine at a friend place so I’ll be back to my laptop later tonight (not sure how) or tomorrow morning.
Hope to give feedback in time 😄

@SergioRAgostinho
Copy link
Member Author

I just pushed a fix for the compilation error #2490.

@claudiofantacci
Copy link
Contributor

claudiofantacci commented Sep 29, 2018

@SergioRAgostinho I pulled the latest master and I set the following options

BUILD_apps_3d_rec_framework      OFF                                                                                 
BUILD_apps_cloud_composer        ON                                                                                 
BUILD_apps_in_hand_scanner       ON                                                                                 
BUILD_apps_modeler               ON                                                                                 
BUILD_apps_optronic_viewer       ON                                                                                 
BUILD_apps_point_cloud_editor    ON

and I'm able to compile PCL just fine 👍 .
However, if I set

BUILD_apps_3d_rec_framework      ON                                                                                 

I get the following errors (several of the following repeted):

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/map:820:5: error: static_assert failed "Allocator::value_type must be same type as value_type"
    static_assert((is_same<typename allocator_type::value_type, value_type>::value),
    ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/Claudio/GitHub/pcl/apps/3d_rec_framework/include/pcl/apps/3d_rec_framework/pipeline/local_recognizer.h:92:53: note: in instantiation of template class 'std::__1::map<std::__1::pair<std::__1::basic_string<char>, int>, Eigen::Matrix<float, 4, 4, 0, 4, 4>, std::__1::less<std::__1::pair<std::__1::basic_string<char>, int> >, Eigen::aligned_allocator<std::__1::pair<std::__1::pair<std::__1::basic_string<char>, int>, Eigen::Matrix<float, 4, 4, 0, 4, 4> > > >' requested here
            std::string, int>, Eigen::Matrix4f> > > poses_cache_;
                                                    ^
In file included from /Users/Claudio/GitHub/pcl/apps/3d_rec_framework/src/pipeline/local_recognizer.cpp:1:
/Users/Claudio/GitHub/pcl/apps/3d_rec_framework/include/pcl/apps/3d_rec_framework/pipeline/impl/local_recognizer.hpp:466:7: error: no template named 'iterator' in 'std::__1::map<std::__1::pair<std::__1::basic_string<char>, int>, Eigen::Matrix<float, 4, 4, 0, 4, 4>, std::__1::less<std::__1::pair<std::__1::basic_string<char>, int> >, Eigen::aligned_allocator<std::__1::pair<std::__1::pair<std::__1::basic_string<char>, int>, Eigen::Matrix<float, 4, 4, 0, 4, 4> > > >'; did you mean 'std::iterator'?
      std::map<mv_pair, Eigen::Matrix4f, std::less<mv_pair>, Eigen::aligned_allocator<std::pair<mv_pair, Eigen::Matrix4f> > >::iterator it =
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      std::iterator
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/iterator:522:29: note: 'std::iterator' declared here
struct _LIBCPP_TEMPLATE_VIS iterator
                            ^
In file included from /Users/Claudio/GitHub/pcl/apps/3d_rec_framework/src/pipeline/local_recognizer.cpp:1:
/Users/Claudio/GitHub/pcl/apps/3d_rec_framework/include/pcl/apps/3d_rec_framework/pipeline/impl/local_recognizer.hpp:466:128: error: use of class template 'std::map<mv_pair, Eigen::Matrix4f, std::less<mv_pair>, Eigen::aligned_allocator<std::pair<mv_pair, Eigen::Matrix4f> > >::iterator' requires template arguments
      std::map<mv_pair, Eigen::Matrix4f, std::less<mv_pair>, Eigen::aligned_allocator<std::pair<mv_pair, Eigen::Matrix4f> > >::iterator it =
                                                                                                                               ^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/iterator:522:29: note: template is declared here
struct _LIBCPP_TEMPLATE_VIS iterator
                            ^
13 warnings and 3 errors generated.
make[2]: *** [apps/3d_rec_framework/CMakeFiles/pcl_3d_rec_framework.dir/src/pipeline/local_recognizer.cpp.o] Error 1
make[1]: *** [apps/3d_rec_framework/CMakeFiles/pcl_3d_rec_framework.dir/all] Error 2

@SergioRAgostinho
Copy link
Member Author

I don't get that compilation error on Ubuntu 18.04. OpenNI was deprecated from homebrew so it's now very cumbersome to set it up.

@claudiofantacci are you capable of giving a shot at fixing that one?

@claudiofantacci
Copy link
Contributor

I have openni for some previous installation requirements, but I have never used it before.

I can try to fix it, but not sure if I will be able to do it in reasonable time.

A sidenote: I can't compile PCL with CUDA and GPU enabled from an XCode project, but I'm able to do it from terminal. Have you ever experienced this before?

@SergioRAgostinho
Copy link
Member Author

I have openni for some previous installation requirements, but I have never used it before.

I don't believe this to be a problem specific to OpenNI. It is something related to static type checking. I'm more convinced that the problem is being triggered from using a recent Eigen version but I can't say for sure.

A sidenote: I can't compile PCL with CUDA and GPU enabled from an XCode project, but I'm able to do it from terminal. Have you ever experienced this before?

Not really. I haven't managed to set up CUDA easily on my OS X environment and gave up at some point.

In the meantime tests io_ply_mesh_io and sample_consensus_line_models should now be running green in master.

@claudiofantacci
Copy link
Contributor

In the meantime tests io_ply_mesh_io and sample_consensus_line_models should now be running green in master.

I'll try that.

Not really. I haven't managed to set up CUDA easily on my OS X environment and gave up at some point.

I know, that's not trivial. I opened an issue on KitWare GitLab here for some other problem I found using CUDA under CMake, but didn't have any feedback yet. I can have a look at it in the future 👍

I don't believe this to be a problem specific to OpenNI. It is something related to static type checking. I'm more convinced that the problem is being triggered from using a recent Eigen version but I can't say for sure.

You're right, I'll have a look into it, but I will have more time tomorrow 😭

@claudiofantacci
Copy link
Contributor

claudiofantacci commented Sep 29, 2018

I have a fix for the Eigen error. Testing it and try to push it asap.

@claudiofantacci
Copy link
Contributor

I opened PR #2495 to fix the error.
Rebasing and running ctest again I have the following result

The following tests FAILED:
	 13 - common_eigen (Failed)
	 24 - feature_rift_estimation (Failed)
	 26 - feature_cppf_estimation (Failed)
	 28 - feature_pfh_estimation (Failed)
	 51 - filters_sampling (Failed)
	 85 - a_registration_test (Failed)

I confirm that io_ply_mesh_io and sample_consensus_line_models are not failing anymore.

@claudiofantacci
Copy link
Contributor

I take the chance to ask: is there a reason to why you don't have a .gitignore file?

@SergioRAgostinho
Copy link
Member Author

We never saw a real need for it. Personally the only thing I felt useful to exclude is the build and install (prefix) folders I create. But since that is only relevant to me and my development habits, I added it to .git/info/exclude.

@claudiofantacci
Copy link
Contributor

I see. My opinion is that something minimal like this would be helpful.

@jspricke
Copy link
Member

.gitignore has been discussed in #745 and #1895 already ;). use ~/.gitignore if you need it.

@SergioRAgostinho
Copy link
Member Author

Can everyone run a new round of tests with the current master ce41673 please? Thank you all for your help :)

@taketwo
Copy link
Member

taketwo commented Oct 12, 2018

Ubuntu 16.04. I've configured with your suggested command (without GPU/CUDA) and successfully built everything. Out of 110 test 2 failed: common_eigen and a_octree_test.

claudiofantacci added a commit to claudiofantacci/pcl that referenced this issue Oct 19, 2018
claudiofantacci added a commit to claudiofantacci/pcl that referenced this issue Oct 19, 2018
The number of thread passed to OpenMP must be greater than 0. The default for OpenMP classes is 0. With this commit, by setting 0, the number of threads is set to the number of cores detected on the machine.

Closes PointCloudLibrary#2568

See also PointCloudLibrary#2399
claudiofantacci added a commit to claudiofantacci/pcl that referenced this issue Oct 19, 2018
The number of thread passed to OpenMP must be greater than 0. The default for OpenMP classes is 0. With this commit, by setting 0, the number of threads is set to the number of cores detected on the machine.

Closes PointCloudLibrary#2568

See also PointCloudLibrary#2399
claudiofantacci added a commit to claudiofantacci/pcl that referenced this issue Oct 19, 2018
claudiofantacci added a commit to claudiofantacci/pcl that referenced this issue Oct 19, 2018
claudiofantacci added a commit to claudiofantacci/pcl that referenced this issue Oct 19, 2018
The number of thread passed to OpenMP must be greater than 0. The default for OpenMP classes is 0. With this commit, by setting 0, the number of threads is set to the number of cores detected on the machine.

Closes PointCloudLibrary#2568

See also PointCloudLibrary#2399
claudiofantacci added a commit to claudiofantacci/pcl that referenced this issue Oct 19, 2018
@claudiofantacci
Copy link
Contributor

An update on Windows tests in Debug configuration.

I checked all of them an proposed a fixes that can be found in:

  1. Test 2: 1.9.0 Release  #2399 (comment)
  2. Test 31 and 80: 1.9.0 Release  #2399 (comment)
  3. Test 82: Test 82 failing under Windows in Debug configuration #2568
  4. Test 85: Test 85 failing under Windows in Debug configuration #2570
  5. Test 106: Test 106 failing under Windows in Debug configuration #2585

The only test that needs further investigation is Test 85 #2570 that may be postponed after this release.

All the fixes can be found in my personal fork here, from 89a3361 and up.

@SergioRAgostinho
Copy link
Member Author

Thanks for detailed layout Claudio and sorry for the absence response. I'm halfway through a relocation (to Italy actually!) and I haven't found a stable environment to have a look at things with time.

@claudiofantacci
Copy link
Contributor

Thanks for detailed layout Claudio and sorry for the absence response. I'm halfway through a relocation (to Italy actually!) and I haven't found a stable environment to have a look at things with time.

I relocated some 4 months ago, I really understand that eheh 😄
From GitHub I see you in Genoa...I'm in Genoa as well! Drop me an email if you want some info 👍

Don't worry for the fixes, we'll have a look as soon as you have some time 👍

@SergioRAgostinho
Copy link
Member Author

SergioRAgostinho commented Nov 6, 2018

1.9.0 is released. 👍

Edit: Also I was gonna propose the next release for early March roughly, as long as the library is stable.

@claudiofantacci
Copy link
Contributor

Great! Thanks @SergioRAgostinho 👍 Awesome work by everyone and terrific final rush to get some fix done 😄

@SergioRAgostinho
Copy link
Member Author

  • Add link doc link to tutorials page

@PointCloudLibrary/maintainers how exactly does one generate documentation for the website for a specific version and add the corresponding link to the docs page? 😅

@taketwo
Copy link
Member

taketwo commented Nov 6, 2018

Will do

@UnaNancyOwen
Copy link
Member

Congrats! 🎉
I will create all-in-one installer in a few days.

@taketwo
Copy link
Member

taketwo commented Nov 7, 2018

Documentation is online. Also, I added instructions to the wiki page.

@UnaNancyOwen
Copy link
Member

I created and uploaded PCL 1.9.0 All-in-one Installer for Visual Studio 2017!

@SergioRAgostinho
Copy link
Member Author

Thank you @taketwo (especially for those instructions) and @UnaNancyOwen . I'll open the PR to bump version to dev again.

@taketwo
Copy link
Member

taketwo commented Nov 8, 2018

I suppose we can close this now?

@UnaNancyOwen
Copy link
Member

UnaNancyOwen commented Nov 8, 2018

I sent a pull request microsoft/vcpkg#4677 to vcpkg to update PCL port to PCL 1.9.0.
If there is no problem, it will be merged soon. It's merged! 🎉

@SergioRAgostinho
Copy link
Member Author

@claudiofantacci says no because we're still missing the party. And I still need to continue with the ML migration to send that announcement email.

@claudiofantacci
Copy link
Contributor

claudiofantacci commented Nov 8, 2018

@claudiofantacci says no because we're still missing the party. And I still need to continue with the ML migration to send that announcement email.

I'm really committed to fulfill the initial checklist.
I propose to close this issue after posting a party picture 🎉 (and of course after taking care of the ML)

@SergioRAgostinho
Copy link
Member Author

I'm opening a 1.9.1 issue to release some important patches we did to cmake so I'll migrate the two opening points to that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants