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

Build tutorials on Azure Pipelines #2696

Merged
merged 10 commits into from
Dec 12, 2018

Conversation

taketwo
Copy link
Member

@taketwo taketwo commented Dec 8, 2018

This PR adds a new Ubuntu-based pipeline "Tutorials" which builds and installs PCL and then tries to build all tutorials as standalone projects. The pipeline will fail in case of configuration or compilation errors, including warnings. (The latter is to catch deprecations.)

Additionally several currently broken tutorials are fixed.

Edit: Edit: Closes 2672 (not anymore).

@taketwo taketwo force-pushed the build-tutorials branch 3 times, most recently from 903bb88 to 3883120 Compare December 9, 2018 10:06
@taketwo
Copy link
Member Author

taketwo commented Dec 9, 2018

OK, not as bad as I expected, 61 out of 72 tutorials can still be built!

2018-12-09T12:26:12.0130467Z Tutorial build summary
2018-12-09T12:26:12.0131061Z ----------------------
2018-12-09T12:26:12.0131286Z 
2018-12-09T12:26:12.0131566Z alignment_prerejective                          🗸  
2018-12-09T12:26:12.0131740Z bare_earth                                      🗸  
2018-12-09T12:26:12.0131928Z bspline_fitting                                 𐄂  build error
2018-12-09T12:26:12.0132115Z cloud_viewer                                    🗸  
2018-12-09T12:26:12.0137919Z cluster_extraction                              🗸  
2018-12-09T12:26:12.0138311Z concatenate_clouds                              🗸  
2018-12-09T12:26:12.0138509Z concatenate_fields                              🗸  
2018-12-09T12:26:12.0138675Z concatenate_points                              🗸  
2018-12-09T12:26:12.0138846Z concave_hull_2d                                 🗸  
2018-12-09T12:26:12.0139039Z conditional_euclidean_clustering                𐄂  build error
2018-12-09T12:26:12.0139214Z conditional_removal                             𐄂  build error
2018-12-09T12:26:12.0139381Z convex_hull_2d                                  🗸  
2018-12-09T12:26:12.0139580Z correspondence_grouping                         🗸  
2018-12-09T12:26:12.0139745Z cylinder_segmentation                           🗸  
2018-12-09T12:26:12.0139918Z davidsdk                                        𐄂  cmake error
2018-12-09T12:26:12.0140109Z don_segmentation                                𐄂  build error
2018-12-09T12:26:12.0140284Z ensenso_cameras                                 𐄂  cmake error
2018-12-09T12:26:12.0140464Z extract_indices                                 🗸  
2018-12-09T12:26:12.0140645Z global_hypothesis_verification                  🗸  
2018-12-09T12:26:12.0140818Z gpu                                             𐄂  cmake error
2018-12-09T12:26:12.0140982Z greedy_projection                               🗸  
2018-12-09T12:26:12.0141171Z ground_based_rgbd_people_detection              𐄂  build error
2018-12-09T12:26:12.0141349Z iccv2011                                        𐄂  build error
2018-12-09T12:26:12.0141516Z implicit_shape_model                            🗸  
2018-12-09T12:26:12.0141693Z interactive_icp                                 🗸  
2018-12-09T12:26:12.0141945Z iros2011                                        🗸  
2018-12-09T12:26:12.0142109Z iterative_closest_point                         🗸  
2018-12-09T12:26:12.0142275Z kdtree_search                                   🗸  
2018-12-09T12:26:12.0142458Z matrix_transform                                🗸  
2018-12-09T12:26:12.0142633Z min_cut_segmentation                            🗸  
2018-12-09T12:26:12.0142971Z model_outlier_removal                           🗸  
2018-12-09T12:26:12.0149654Z moment_of_inertia                               🗸  
2018-12-09T12:26:12.0149854Z narf_descriptor_visualization                   🗸  
2018-12-09T12:26:12.0150042Z narf_feature_extraction                         🗸  
2018-12-09T12:26:12.0150213Z narf_keypoint_extraction                        🗸  
2018-12-09T12:26:12.0150378Z normal_distributions_transform                  🗸  
2018-12-09T12:26:12.0150564Z normal_estimation_using_integral_images         🗸  
2018-12-09T12:26:12.0150750Z octree_change_detection                         🗸  
2018-12-09T12:26:12.0150914Z octree_search                                   🗸  
2018-12-09T12:26:12.0151079Z openni_grabber                                  🗸  
2018-12-09T12:26:12.0151261Z openni_narf_keypoint_extraction                 🗸  
2018-12-09T12:26:12.0151437Z openni_range_image_visualization                🗸  
2018-12-09T12:26:12.0151602Z pairwise_incremental_registration               🗸  
2018-12-09T12:26:12.0151786Z passthrough                                     🗸  
2018-12-09T12:26:12.0151952Z pcd_read                                        🗸  
2018-12-09T12:26:12.0152116Z pcd_write                                       🗸  
2018-12-09T12:26:12.0152477Z pcl_painter2D                                   𐄂  build error
2018-12-09T12:26:12.0152649Z pcl_plotter                                     𐄂  build error
2018-12-09T12:26:12.0153579Z pcl_visualizer                                  🗸  
2018-12-09T12:26:12.0153830Z planar_segmentation                             🗸  
2018-12-09T12:26:12.0154047Z point_cloud_compression                         🗸  
2018-12-09T12:26:12.0154261Z project_inliers                                 🗸  
2018-12-09T12:26:12.0154491Z qt_colorize_cloud                               🗸  
2018-12-09T12:26:12.0154835Z qt_visualizer                                   🗸  
2018-12-09T12:26:12.0155051Z radius_outlier_removal                          🗸  
2018-12-09T12:26:12.0155281Z random_sample_consensus                         🗸  
2018-12-09T12:26:12.0155498Z range_image_border_extraction                   🗸  
2018-12-09T12:26:12.0155714Z range_image_creation                            🗸  
2018-12-09T12:26:12.0155929Z range_image_visualization                       🗸  
2018-12-09T12:26:12.0156165Z region_growing_rgb_segmentation                 🗸  
2018-12-09T12:26:12.0156426Z region_growing_segmentation                     🗸  
2018-12-09T12:26:12.0156658Z registration_api                                🗸  
2018-12-09T12:26:12.0157032Z remove_outliers                                 🗸  
2018-12-09T12:26:12.0157194Z resampling                                      🗸  
2018-12-09T12:26:12.0157354Z rops_feature                                    🗸  
2018-12-09T12:26:12.0157532Z statistical_removal                             🗸  
2018-12-09T12:26:12.0157703Z stick_segmentation                              🗸  
2018-12-09T12:26:12.0158036Z supervoxel_clustering                           🗸  
2018-12-09T12:26:12.0158220Z template_alignment                              🗸  
2018-12-09T12:26:12.0158388Z tracking                                        🗸  
2018-12-09T12:26:12.0158554Z vfh_recognition                                 🗸  
2018-12-09T12:26:12.0158734Z voxel_grid                                      🗸  
2018-12-09T12:26:12.0158760Z 
2018-12-09T12:26:12.0158797Z Succeeded building 61 out of 72 tutorials

Unfortunately, this adds another 25+ minutes to our Ubuntu pipeline. Question is, do we really need this to run on every pull request or merge? AFAIU Azure supports pipelines that are inactive by default, but can be triggered manually or periodically. Perhaps we can opt for this.

@SergioRAgostinho
Copy link
Member

SergioRAgostinho commented Dec 9, 2018

Great job.

AFAIU Azure supports pipelines that are inactive by default, but can be triggered manually or periodically. Perhaps we can opt for this.

Agree with both modalities. There are some PRs where we clearly know it will affect downstream targets so the manual option is important here. The periodic check (once a week?) will also grant us some level of reassurance.

@taketwo taketwo force-pushed the build-tutorials branch 2 times, most recently from dcbb7f8 to 2d10543 Compare December 9, 2018 22:04
@taketwo
Copy link
Member Author

taketwo commented Dec 10, 2018

I have dived into Azure documentation and came up with 4 ways to incorporate tutorial builds into CI.

  1. Add it as a step in the "Build.Ubuntu" pipeline. Simple. Increases pipeline time by 25+ minutes.
  2. Add it as a separate pipeline.
    2a. Inactive, can be manually triggered. Needs to build PCL from scratch, so will take 2+ hours to complete.
    2b. Inactive, automatically triggered by successful builds of "Build.Ubuntu", receives built library binaries from it, so needs only 25+ minutes to complete.
    2c. Active, always runs. Needs to build PCL from scratch, so will take 2+ hours to complete.

1 is long.

2a is also long, plus manual triggering for pull requests is awkward, you need to copy commit ID.

2b seemed to be the best. The time to complete the checks is not increased, but if we have reasons to believe that PR may affect tutorials, we wait additional 30 minutes until that extra pipeline is completed. Unfortunately, turned out to be impossible, because "build triggers" don't work for PR. So it would only be possible to run this additional pipeline on merges to master, which is too late to catch problems.

2c is like 1, long and even occupies more resources. However, I had an idea that we can set PCL_NO_PRECOMPILE=ON, so that we don't spend much time compiling library. I tried this approach and it kind of works. Here is the breakdown of the "Tutorials" pipeline build:

image

Library is done in 25 minutes, plus tutorials take another 40. So in the end the pipeline a bit over 1 hour, which means it's actually faster than the rest of "Build.xxx" pipelines.

Seems like the best approach to me now. The only downside is that we are occupying one more build agent.

@SergioRAgostinho
Copy link
Member

Thank you for the detailed breakdown. I would say at this point is hard to make a decisions based on the inconveniences having one more build agent, so I would kick in with option 2c as you suggested, especially because everything else seems to come with worse inconveniences.

@taketwo
Copy link
Member Author

taketwo commented Dec 11, 2018

OK.

One other thing that I noticed is that not all of the existing tutorials are included in the TOC. For example, "Removing outliers using a ConditionalRemoval filter" is not on the tutorials webpage. We need to go through this at some point and either include in the list or delete such dangling tutorials.

As for this PR, the remaining work items are:

I'm actually not sure anymore if this should close #2672. In it's current state this pipeline only builds tutorials on Ubuntu, so we can not be sure that downstream projects can configure/build against PCL on other platforms.

@SergioRAgostinho
Copy link
Member

I'm actually not sure anymore if this should close #2672. In it's current state this pipeline only builds tutorials on Ubuntu, so we can not be sure that downstream projects can configure/build against PCL on other platforms.

We can always have an extra step like you did in 1 where we build just a single target, filled with one class from each module. We compile and run the executable to ensure things don't segfault magically.

Copy link
Member

@SergioRAgostinho SergioRAgostinho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still a WIP? Looks pretty finished. There's some lines which are not following the style guide, but I'm assuming you did that to keep consistency with the surrounding code.

@taketwo taketwo changed the title [WIP] Build tutorials on Azure Pipelines Build tutorials on Azure Pipelines Dec 12, 2018
@taketwo
Copy link
Member Author

taketwo commented Dec 12, 2018

Yep, this is ready now. The style is left as-is for consistency.

In my latest commit I enabled -Werror to make sure that we notice when we deprecate a function that is used in tutorials. (Most of the fixes I had to apply were about long ago deprecated functions.)

@SergioRAgostinho SergioRAgostinho merged commit 5cef903 into PointCloudLibrary:master Dec 12, 2018
@taketwo taketwo deleted the build-tutorials branch December 12, 2018 09:48
@SergioRAgostinho
Copy link
Member

Should we add some fancy badges for the docs and tutorials on the README.md?

@taketwo
Copy link
Member Author

taketwo commented Dec 12, 2018

If you want... :)

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

Successfully merging this pull request may close these issues.

2 participants