Skip to content

Fix ParallelSGD test step sizes and enable OpenMP for tests when found #454

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

Merged
rcurtin merged 5 commits into from
Apr 18, 2026
Merged

Fix ParallelSGD test step sizes and enable OpenMP for tests when found #454

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8312a65
If we find OpenMP, use it.
rcurtin Apr 14, 2026
4c7ba83
Fix unused typedef warning.
rcurtin Apr 14, 2026
ba6a278
Adjust step sizes so that tests converge for ParallelSGD.
rcurtin Apr 14, 2026
1d180d8
Include OpenMP explicitly if ENS_USE_OPENMP is defined.
rcurtin Apr 14, 2026
fd4362d
Use target_compile_definitions() instead.
rcurtin Apr 17, 2026
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: 3 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ endif()

# Find OpenMP and link it.
if(USE_OPENMP)
find_package(OpenMP)
if(NOT TARGET OpenMP::OpenMP_CXX)
find_package(Threads REQUIRED)
add_library(OpenMP::OpenMP_CXX IMPORTED INTERFACE)
Expand All @@ -61,6 +62,8 @@ if(USE_OPENMP)
set_property(TARGET OpenMP::OpenMP_CXX
PROPERTY INTERFACE_LINK_LIBRARIES ${OpenMP_CXX_FLAGS} Threads::Threads)
endif()

add_definitions(-DENS_USE_OPENMP)

@Segfault1602 Segfault1602 Apr 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should it be target_compile_definition instead?

@rcurtin rcurtin Apr 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks---my CMake is often pretty primitive. Fixed in fd4362d.

target_link_libraries(ensmallen INTERFACE OpenMP::OpenMP_CXX)
endif()

Expand Down
1 change: 1 addition & 0 deletions include/ensmallen_bits/config.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#define ENS_PRAGMA_OMP_ATOMIC _Pragma("omp atomic")
#define ENS_PRAGMA_OMP_CRITICAL _Pragma("omp critical")
#define ENS_PRAGMA_OMP_CRITICAL_NAMED _Pragma("omp critical(section)")
#include <omp.h>
#else
#define ENS_PRAGMA_OMP_PARALLEL
#define ENS_PRAGMA_OMP_ATOMIC
Expand Down
3 changes: 3 additions & 0 deletions tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ set(ENSMALLEN_TESTS_SOURCES
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR})
add_executable(ensmallen_tests EXCLUDE_FROM_ALL ${ENSMALLEN_TESTS_SOURCES})
target_link_libraries(ensmallen_tests PRIVATE ensmallen)
if (USE_OPENMP)
target_link_libraries(ensmallen_tests PRIVATE OpenMP::OpenMP_CXX)
endif ()

# Copy test data into place.
add_custom_command(TARGET ensmallen_tests
Expand Down
4 changes: 1 addition & 3 deletions tests/moead_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,8 +560,6 @@ TEMPLATE_TEST_CASE("DefaultMOEAD_ZDT1Function", "[MOEAD]", ENS_TEST_TYPES)
*/
TEMPLATE_TEST_CASE("DirichletMOEAD_ZDT3Function", "[MOEAD]", ENS_TEST_TYPES)
{
typedef typename TestType::elem_type ElemType;

//! Parameters taken from original ZDT Paper.
ZDT3<TestType> zdt3(300);
const double lowerBound = 0;
Expand Down Expand Up @@ -591,4 +589,4 @@ TEMPLATE_TEST_CASE("DirichletMOEAD_ZDT3Function", "[MOEAD]", ENS_TEST_TYPES)
opt.Optimize(objectives, coords, finalPopulation, finalFront);

REQUIRE(VariableBoundsCheck(finalPopulation));
}
}
12 changes: 6 additions & 6 deletions tests/parallel_sgd_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ TEMPLATE_TEST_CASE("ParallelSGDTest_SparseFunction", "[ParallelSGD]",

size_t batchSize = std::ceil((float) f.NumFunctions() / i);

ConstantStep decayPolicy(0.4 * batchSize);
ParallelSGD<ConstantStep> s(10000, batchSize, 1e-5, true, decayPolicy);
ConstantStep decayPolicy(0.02 * batchSize);
ParallelSGD<ConstantStep> s(10000, batchSize, 1e-7, true, decayPolicy);
FunctionTest<SparseTestFunction>(s,
Tolerances<TestType>::LargeObj,
Tolerances<TestType>::LargeCoord);
Expand All @@ -62,22 +62,22 @@ TEMPLATE_TEST_CASE("ParallelSGD_GeneralizedRosenbrockFunction",
// Create the generalized Rosenbrock function.
GeneralizedRosenbrockFunction f(i);

ConstantStep decayPolicy(0.001 * f.NumFunctions());
ConstantStep decayPolicy(0.00005 * f.NumFunctions());

ParallelSGD<ConstantStep> s(
100000, f.NumFunctions(), Tolerances<TestType>::Obj / 100, true,
1000000, f.NumFunctions(), Tolerances<TestType>::Obj / 100, true,
decayPolicy);

TestType coordinates = f.GetInitialPoint<TestType>();

omp_set_num_threads(1);
double result = s.Optimize(f, coordinates);

REQUIRE(result == Approx(0.0).margin(Tolerances<TestType>::Obj));
REQUIRE(result == Approx(0.0).margin(100 * Tolerances<TestType>::Obj));
for (size_t j = 0; j < i; ++j)
{
REQUIRE(coordinates(j) ==
Approx(1.0).epsilon(Tolerances<TestType>::Coord));
Approx(1.0).epsilon(100 * Tolerances<TestType>::Coord));
}
}
}
Expand Down
Loading