Skip to content

Replace omp_set_nested (deprecated) with omp_set_max_active_levels #1887

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Replace omp_set_nested (deprecated) with omp_set_max_active_levels #1887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.25)
cmake_minimum_required(VERSION 3.30)
message("CMake version: ${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION}")

# ==============================================================================
Expand Down Expand Up @@ -253,6 +253,7 @@ endif()
if (ALICEVISION_USE_OPENMP STREQUAL "OFF")
set(ALICEVISION_HAVE_OPENMP 0)
else() # ON OR AUTO
set(OpenMP_RUNTIME_MSVC "llvm")
find_package(OpenMP)

if (OPENMP_FOUND)
Expand Down
2 changes: 1 addition & 1 deletion src/aliceVision/alicevision_omp.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ inline int omp_get_thread_num() { return 0; }
inline int omp_get_max_threads() { return 1; }
inline void omp_set_num_threads(int num_threads) {}
inline int omp_get_num_procs() { return 1; }
inline void omp_set_nested(int nested) {}
inline void omp_set_max_active_levels(int levels) {}

inline void omp_init_lock(omp_lock_t* lock) {}
inline void omp_destroy_lock(omp_lock_t* lock) {}
Expand Down
2 changes: 1 addition & 1 deletion src/aliceVision/featureEngine/FeatureExtractor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ void FeatureExtractor::process(const HardwareContext& hContext, const image::EIm
nbThreads = std::min(cpuJobs.size(), nbThreads);

ALICEVISION_LOG_INFO("# threads for extraction: " << nbThreads);
omp_set_nested(1);
omp_set_max_active_levels(2);
Copy link

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

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

[nitpick] Again, consider using a named constant or comment for 2 to make its intent clear in this context of nested parallel extraction.

Suggested change
omp_set_max_active_levels(2);
// Set the maximum number of nested parallel levels for OpenMP.
const int MAX_ACTIVE_LEVELS = 2;
omp_set_max_active_levels(MAX_ACTIVE_LEVELS);

Copilot uses AI. Check for mistakes.


#pragma omp parallel for num_threads(nbThreads)
for (int i = 0; i < cpuJobs.size(); ++i)
Expand Down
8 changes: 4 additions & 4 deletions src/aliceVision/fuseCut/PointCloud.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ void createVerticesWithVisibilities(const StaticVector<int>& cams,
for (auto& lock : locks)
omp_init_lock(&lock);

omp_set_nested(1);
omp_set_max_active_levels(2);
Copy link

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider extracting the magic number 2 into a named constant (e.g. kMaxOmpActiveLevels) or adding a comment to explain why two levels are required, improving readability and easing future adjustments.

Suggested change
omp_set_max_active_levels(2);
constexpr int kMaxOmpActiveLevels = 2; // Maximum number of nested parallel regions allowed by OpenMP.
omp_set_max_active_levels(kMaxOmpActiveLevels);

Copilot uses AI. Check for mistakes.

#pragma omp parallel for num_threads(3)
for (int c = 0; c < cams.size(); ++c)
{
Expand Down Expand Up @@ -229,7 +229,7 @@ void createVerticesWithVisibilities(const StaticVector<int>& cams,
}
}
}
omp_set_nested(0);
omp_set_max_active_levels(1);
Copy link

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

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

[nitpick] Similarly, you may want to define 1 as a constant (e.g. kSingleOmpLevel) to clarify that this disables nested parallelism and maintain consistency.

Suggested change
omp_set_max_active_levels(1);
omp_set_max_active_levels(kSingleOmpLevel);

Copilot uses AI. Check for mistakes.


for (auto& lock : locks)
omp_destroy_lock(&lock);
Expand Down Expand Up @@ -294,7 +294,7 @@ void PointCloud::fuseFromDepthMaps(const StaticVector<int>& cams, const Point3d

ALICEVISION_LOG_INFO("Load depth maps and add points.");
{
omp_set_nested(1);
omp_set_max_active_levels(2);
Copy link

Copilot AI Jul 2, 2025

Choose a reason for hiding this comment

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

[nitpick] This duplicated literal could also be replaced by a shared constant or inline comment, so readers understand it matches the upper-level parallel region's configuration.

Suggested change
omp_set_max_active_levels(2);
omp_set_max_active_levels(MAX_OMP_ACTIVE_LEVELS);

Copilot uses AI. Check for mistakes.

#pragma omp parallel for num_threads(3)
for (int c = 0; c < cams.size(); c++)
{
Expand Down Expand Up @@ -424,7 +424,7 @@ void PointCloud::fuseFromDepthMaps(const StaticVector<int>& cams, const Point3d
}
}
}
omp_set_nested(0);
omp_set_max_active_levels(1);
}

ALICEVISION_LOG_INFO("Filter initial 3D points by pixel size to remove duplicates.");
Expand Down
Loading