Skip to content

Commit

Permalink
Merge pull request #67 from lan496/goodbye-openmp
Browse files Browse the repository at this point in the history
Remove OpenMP support
  • Loading branch information
lan496 committed Apr 23, 2024
2 parents 95f2d6c + 9bb16f0 commit b26bc9f
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 41 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/cpp.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ on:
paths:
- '**.cpp'
- '**.hpp'
pull_request:
branches: [main, develop]
paths:
- '**.cpp'
- '**.hpp'
workflow_dispatch:

jobs:
test-tdzdd:
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ name: testing
on:
push:
branches: [main, develop]
pull_request:
branches: [main, develop]
workflow_dispatch:

jobs:
tests:
Expand Down
13 changes: 0 additions & 13 deletions xtal_tdzdd/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,6 @@ FetchContent_MakeAvailable(tdzdd)
add_library(xtal_tdzdd STATIC
iterator.cpp)

# https://cliutils.gitlab.io/modern-cmake/chapters/packages/OpenMP.html
# https://iscinumpy.gitlab.io/post/omp-on-high-sierra/
find_package(OpenMP REQUIRED)
if(NOT TARGET OpenMP::OpenMP_CXX)
add_library(OpenMP_TARGET INTERFACE)
add_library(OpenMP::OpenMP_CXX ALIAS OpenMP_TARGET)
target_compile_options(OpenMP_TARGET INTERFACE ${OpenMP_CXX_FLAGS})
find_package(Threads REQUIRED)
target_link_libraries(OpenMP_TARGET INTERFACE Threads::Threads)
target_link_libraries(OpenMP_TARGET INTERFACE ${OpenMP_CXX_FLAGS})
endif()
target_link_libraries(xtal_tdzdd PUBLIC OpenMP::OpenMP_CXX)

target_compile_options(xtal_tdzdd PRIVATE
-O3 -Wall -Winit-self -Wlogical-op)
set_target_properties(xtal_tdzdd PROPERTIES POSITION_INDEPENDENT_CODE ON)
Expand Down
22 changes: 9 additions & 13 deletions xtal_tdzdd/short_range_order.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ void prepare_binary_derivative_structures_with_sro_(
const std::vector<graph::Vertex>& vertex_order,
const std::vector<permutation::Permutation>& automorphism,
const std::vector<std::pair<std::vector<int>, int>>&
composition_constraints,
bool useMP) {
composition_constraints) {
// sanity check
for (const auto& perm : automorphism) {
if (perm.get_size() != static_cast<size_t>(num_sites)) {
Expand Down Expand Up @@ -144,7 +143,7 @@ void prepare_binary_derivative_structures_with_sro_(

// ======== construct DD ========
dd = universe::Universe(num_variables);
dd.useMultiProcessors(useMP);
dd.useMultiProcessors(false);

// spec for composition constraints
assert(!composition_constraints.empty());
Expand Down Expand Up @@ -179,8 +178,7 @@ void prepare_multicomponent_derivative_structures_with_sro_(
const std::vector<graph::Vertex>& vertex_order,
const std::vector<permutation::Permutation>& automorphism,
const std::vector<std::pair<std::vector<int>, int>>&
composition_constraints,
bool useMP) {
composition_constraints) {
size_t num_variables = num_sites * num_types;
// sanity check
for (const auto& perm : automorphism) {
Expand Down Expand Up @@ -215,7 +213,7 @@ void prepare_multicomponent_derivative_structures_with_sro_(

// ======== construct DD ========
dd = universe::Universe(num_variables);
dd.useMultiProcessors(useMP);
dd.useMultiProcessors(false);

// spec for one-of-k
for (permutation::Element site = 0;
Expand Down Expand Up @@ -271,8 +269,7 @@ void prepare_derivative_structures_with_sro(
const std::vector<graph::Vertex>& vertex_order,
const std::vector<permutation::Permutation>& automorphism,
const std::vector<std::pair<std::vector<int>, int>>&
composition_constraints,
bool useMP) {
composition_constraints) {
// sanity check
assert(num_sites >= 1);
assert(num_types >= 2);
Expand All @@ -283,11 +280,11 @@ void prepare_derivative_structures_with_sro(
if (num_types == 2) {
prepare_binary_derivative_structures_with_sro_(
dd, num_sites, num_types, vertex_order, automorphism,
composition_constraints, useMP);
composition_constraints);
} else {
prepare_multicomponent_derivative_structures_with_sro_(
dd, num_sites, num_types, vertex_order, automorphism,
composition_constraints, useMP);
composition_constraints);
}
tdzdd::MessageHandler::showMessages(false);
}
Expand Down Expand Up @@ -336,12 +333,11 @@ void construct_derivative_structures_with_sro(
const std::vector<std::pair<std::vector<int>, int>>&
composition_constraints,
const std::vector<graph::Graph>& graphs,
const std::vector<graph::Weight>& targets, bool remove_superperiodic,
bool useMP) {
const std::vector<graph::Weight>& targets, bool remove_superperiodic) {
// ==== construct DD ====
prepare_derivative_structures_with_sro(dd, num_sites, num_types,
vertex_order, automorphism,
composition_constraints, useMP);
composition_constraints);

// fix SRO
// TODO: better vertex_order choice
Expand Down
6 changes: 0 additions & 6 deletions xtal_tdzdd/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ include_directories(${tdzdd_SOURCE_DIR}/include)

include_directories(../)

# OpenMP
find_package(OpenMP)
if (OPENMP_FOUND)
set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
endif()

add_compile_options(-O3 -Wall -Winit-self -Wlogical-op -fsanitize=address)
add_link_options(-fsanitize=address)

Expand Down
7 changes: 3 additions & 4 deletions xtal_tdzdd/test/test_sro.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ TEST(SroTest, BinaryTest) {
construct_derivative_structures_with_sro(
dd, num_sites, num_types, vertex_order, automorphism, translations,
composition_constraints, graphs, targets,
true, // remove superperiodic
false // not useMP
true // remove superperiodic
);

std::string cardinality_expect = "1";
Expand Down Expand Up @@ -184,8 +183,8 @@ TEST(SroTest, TernaryTest) {
construct_derivative_structures_with_sro(
dd, num_sites, num_types, vertex_order, automorphism, translations,
composition_constraints, cluster_graphs, targets,
true, // remove superperiodic
false);
true // remove superperiodic
);

// return labelings
auto converter = VertexConverter(num_variables, vertex_order);
Expand Down
9 changes: 4 additions & 5 deletions xtal_tdzdd/wrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ PYBIND11_MODULE(pyzdd_, m) {
// DD structure
py::class_<tdzdd::DdStructure<2>> PyDdStructure2(m, "Universe");
PyDdStructure2.def(py::init<>());
PyDdStructure2.def(py::init<int, bool>(), "Universe DD construction",
py::arg("n"), py::arg("useMP") = false);
PyDdStructure2.def(py::init<int>(), "Universe DD construction",
py::arg("n"));
PyDdStructure2.def(
py::init<tdzdd::DdStructure<2> const &>()); // copy constructor
PyDdStructure2.def(py::self == py::self); // operator==
Expand Down Expand Up @@ -115,7 +115,7 @@ PYBIND11_MODULE(pyzdd_, m) {
&pyzdd::derivative_structure::prepare_derivative_structures_with_sro,
py::arg("dd"), py::arg("num_sites"), py::arg("num_types"),
py::arg("vertex_order"), py::arg("automorphism"),
py::arg("composition_constraints"), py::arg("useMP") = false);
py::arg("composition_constraints"));
m.def("restrict_pair_correlation",
&pyzdd::derivative_structure::restrict_pair_correlation,
py::arg("dd"), py::arg("num_sites"), py::arg("num_types"),
Expand All @@ -130,8 +130,7 @@ PYBIND11_MODULE(pyzdd_, m) {
py::arg("dd"), py::arg("num_sites"), py::arg("num_types"),
py::arg("vertex_order"), py::arg("automorphism"),
py::arg("translations"), py::arg("composition_constraints"),
py::arg("graphs"), py::arg("targets"), py::arg("remove_superperiodic"),
py::arg("useMP") = false);
py::arg("graphs"), py::arg("targets"), py::arg("remove_superperiodic"));
// converter
py::class_<pyzdd::derivative_structure::VertexConverter>(
m, "BinaryVertexConverter")
Expand Down

0 comments on commit b26bc9f

Please sign in to comment.