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

FindTBB.cmake: only search TBB in Config mode if version >= 2019 update 5 #5732

Conversation

janetournois
Copy link
Member

Summary of Changes

Avoid the following error when TBB was already found a few lines earlier, by adding a condition NOT TBB_FOUND before find_package(TBB) :

CMake Error at C:/dev/tbb2019_20181203oss_win/tbb2019_20181203oss/cmake/TBBConfig.cmake:83 (add_library):
  add_library cannot create imported target "TBB::tbb" because another target
  with the same name already exists.

Release Management

  • Affected package(s): Polyhedron (demo)
  • License and copyright ownership: unchanged

@lrineau
Copy link
Member

lrineau commented May 27, 2021

I think we should fix #5687, instead. That is a bug from TBB before version 2019 update 5, and we have an easy workaround: #5693 (comment).

@lrineau lrineau added the Not yet approved The feature or pull-request has not yet been approved. label May 27, 2021
@lrineau lrineau self-assigned this May 27, 2021
Laurent said :
"That is a bug in earlier versions of TBB. It was fixed in TBB 2019 update 5. That is a bad interaction with CGAL#5687, that made the module FindTBB.cmake from CGAL search for TBB in Config mode, first."
@janetournois janetournois force-pushed the Polyhedron_demo-fix_cmakelists_find_tbb-jtournois branch from 335449d to 8d4f147 Compare May 27, 2021 14:07
@lrineau lrineau removed the Not yet approved The feature or pull-request has not yet been approved. label May 27, 2021
@lrineau lrineau changed the title Polyhedron demo CMakeLists - do not look for TBB if already found FindTBB.cmake: only search TBB in Config mode if version >= 2019 update 5. May 27, 2021
@lrineau lrineau changed the title FindTBB.cmake: only search TBB in Config mode if version >= 2019 update 5. FindTBB.cmake: only search TBB in Config mode if version >= 2019 update 5 May 27, 2021
@janetournois
Copy link
Member Author

Be careful : CMake is fixed but the demo does not compile yet with this fix

@lrineau
Copy link
Member

lrineau commented May 27, 2021

Be careful : CMake is fixed but the demo does not compile yet with this fix

The compilation error is:

.../Mesh_3/include/CGAL/Mesh_complex_3_in_triangulation_3.h:105:3: error: use of deleted function 'tbb::detail::d1::tbb_hash_compare<std::pair<CGAL::internal::CC_iterator<CGAL::Compact_container<CGAL::Mesh_vertex_3<CGAL::Robust_weighted_circumcenter_filtered_traits_3<CGAL::Mesh_3::Robust_intersection_traits_3_new<CGAL::Epick> >, std::tuple<int, int, int, int>, int, CGAL::Regular_triangulation_vertex_base_3<CGAL::Robust_weighted_circumcenter_filtered_traits_3<CGAL::Mesh_3::Robust_intersection_traits_3_new<CGAL::Epick> >, CGAL::Triangulation_ds_vertex_base_3<CGAL::Triangulation_data_structure_3<CGAL::Mesh_vertex_generator_3<CGAL::Robust_weighted_circumcenter_filtered_traits_3<CGAL::Mesh_3::Robust_intersection_traits_3_new<CGAL::Epick> >, std::tuple<int, int, int, int>, int, CGAL::Regular_triangulation_vertex_base_3<CGAL::Robust_weighted_circumcenter_filtered_traits_3<CGAL::Mesh_3::Robust_intersection_traits_3_new<CGAL::Epick> >, CGAL::Triangulation_ds_vertex_base_3<void> > >, CGAL::Triangulation_cell_base_with_info_3<int, CGAL::Robust_weighted_circumcenter_filtered_traits_3<CGAL::Mesh_3::Robust_intersection_traits_3_new<CGAL::Epick> >, CGAL::Compact_mesh_cell_base_3<CGAL::Robust_weighted_circumcenter_filtered_traits_3<CGAL::Mesh_3::Robust_intersection_traits_3_new<CGAL::Epick> >, CGAL::Polyhedral_mesh_domain_with_features_3<CGAL::Epick, CGAL::Surface_mesh<CGAL::Point_3<CGAL::Epick> >, CGAL::Default, int> > >, CGAL::Sequential_tag> > > >, CGAL::Default, CGAL::Default, CGAL::Default>, false>, CGAL::internal::CC_iterator<CGAL::Compact_container<CGAL::Mesh_vertex_3<CGAL::Robust_weighted_circumcenter_filtered_traits_3<CGAL::Mesh_3::Robust_intersection_traits_3_new<CGAL::Epick> >, std::tuple<int, int, int, int>, int, CGAL::Regular_triangulation_vertex_base_3<CGAL::Robust_weighted_circumcenter_filtered_traits_3<CGAL::Mesh_3::Robust_intersection_traits_3_new<CGAL::Epick> >, CGAL::Triangulation_ds_vertex_base_3<CGAL::Triangulation_data_structure_3<CGAL::Mesh_vertex_generator_3<CGAL::Robust_weighted_circumcenter_filtered_traits_3<CGAL::Mesh_3::Robust_intersection_traits_3_new<CGAL::Epick> >, std::tuple<int, int, int, int>, int, CGAL::Regular_triangulation_vertex_base_3<CGAL::Robust_weighted_circumcenter_filtered_traits_3<CGAL::Mesh_3::Robust_intersection_traits_3_new<CGAL::Epick> >, CGAL::Triangulation_ds_vertex_base_3<void> > >, CGAL::Triangulation_cell_base_with_info_3<int, CGAL::Robust_weighted_circumcenter_filtered_traits_3<CGAL::Mesh_3::Robust_intersection_traits_3_new<CGAL::Epick> >, CGAL::Compact_mesh_cell_base_3<CGAL::Robust_weighted_circumcenter_filtered_traits_3<CGAL::Mesh_3::Robust_intersection_traits_3_new<CGAL::Epick> >, CGAL::Polyhedral_mesh_domain_with_features_3<CGAL::Epick, CGAL::Surface_mesh<CGAL::Point_3<CGAL::Epick> >, CGAL::Default, int> > >, CGAL::Sequential_tag> > > >, CGAL::Default, CGAL::Default, CGAL::Default>, false> > >::~tbb_hash_compare()'

That is because we have

  typedef tbb::concurrent_hash_map<Pair_of_vertices, int> Edge_facet_counter;

where Pair_of_vertices is a std::pair of Vertex_handle. For TBB before oneAPI, we define an overload of tbb_hasher, that can be found by ADL:

But oneAPI-TBB has switched to the use of std::hash.

In think we should pass explicitly a HashCompare to the concurrent_hash_map. That API is the same for TBB and oneAPI-TBB.

In TBB, the default argument for HashCompare was:

template<typename Key>
struct tbb_hash_compare {
    static size_t hash( const Key& a ) { return tbb_hasher(a); }
    static bool equal( const Key& a, const Key& b ) { return a == b; }
};

(see the ADL call to tbb_hasher).

In oneAPI-TBB, that is:

template <typename Key>
class tbb_hash_compare {
public:
    std::size_t hash( const Key& a ) const { return my_hash_func(a); }
    bool equal( const Key& a, const Key& b ) const { return my_key_equal(a, b); }
private:
    std::hash<Key> my_hash_func;
    std::equal_to<Key> my_key_equal;
};

(see the use of std::hash).

…ake variable)

and fix 2019 Update 5 version number
Co-authored-by: Laurent Rineau <Laurent.Rineau@cgal.org>
@maxGimeno
Copy link
Contributor

@lrineau lrineau modified the milestones: 5.1.4, 5.1.5 Jun 2, 2021
@lrineau lrineau added rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' and removed Ready to be tested Under Testing labels Jun 2, 2021
@lrineau lrineau added the rm only: ready for release branch For the release team only: that indicates that a PR is about to be merged in a release branch label Jun 2, 2021
@lrineau lrineau merged commit 9e7d8ab into CGAL:5.1.x-branch Jun 2, 2021
@lrineau lrineau removed rm only: ready for release branch For the release team only: that indicates that a PR is about to be merged in a release branch rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' labels Jun 2, 2021
@lrineau lrineau deleted the Polyhedron_demo-fix_cmakelists_find_tbb-jtournois branch June 2, 2021 15:20
lrineau added a commit that referenced this pull request Jun 2, 2021
…ists_find_tbb-jtournois

FindTBB.cmake: only search TBB in Config mode if version >= 2019 update 5
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.

3 participants