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

Refactor discretization. #2415

Merged
merged 16 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 3 additions & 5 deletions .github/workflows/sanitize.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@ permissions:
jobs:
build:
name: "Sanitize"
runs-on: ubuntu-22.04
runs-on: ubuntu-24.04
strategy:
fail-fast: false
matrix:
name: ["Sanitize"]
sanitizer: ["address", "undefined", "thread"]
simd: ["ON", "OFF"]
env:
CC: clang-14
CXX: clang++-14
CC: clang-18
CXX: clang++-18
ASAN_OPTIONS: detect_leaks=1
steps:
- name: Get build dependencies
Expand All @@ -43,8 +43,6 @@ jobs:
uses: actions/checkout@v4
with:
submodules: recursive
- name: Update pip
run: python -m pip install --upgrade pip
# figure out vector extensions for ccache key
- name: Check vector extensions
run: |
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/test-matrix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ jobs:
- {
name: "Linux Min Clang",
os: "ubuntu-22.04",
cc: "clang-12",
cxx: "clang++-12",
cc: "clang-13",
cxx: "clang++-13",
py: "3.9",
cmake: "3.19.x",
mpi: "ON",
Expand Down
16 changes: 15 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ install(FILES mechanisms/BuildModules.cmake DESTINATION ${ARB_INSTALL_DATADIR})

# First make ourselves less chatty
set(_saved_CMAKE_MESSAGE_LOG_LEVEL ${CMAKE_MESSAGE_LOG_LEVEL})
set(CMAKE_MESSAGE_LOG_LEVEL WARNING)
set(CMAKE_MESSAGE_LOG_LEVEL STATUS)

# in the event we can find hwloc, just add it
find_package(hwloc QUIET)
Expand Down Expand Up @@ -344,6 +344,8 @@ CPMAddPackage(NAME fmt
VERSION 10.0.0
GIT_TAG 10.0.0)

add_library(ext-gtest INTERFACE)
add_library(ext-bench INTERFACE)
if (BUILD_TESTING)
CPMAddPackage(NAME benchmark
GITHUB_REPOSITORY google/benchmark
Expand All @@ -354,6 +356,18 @@ if (BUILD_TESTING)
GIT_TAG release-1.12.1
VERSION 1.12.1
OPTIONS "INSTALL_GTEST OFF" "BUILD_GMOCK OFF")
if(benchmark_ADDED)
target_link_libraries(ext-bench INTERFACE benchmark)
else()
find_package(benchmark REQUIRED)
target_link_libraries(ext-bench INTERFACE benchmark::benchmark)
endif()
if(googletest_ADDED)
target_link_libraries(ext-gtest INTERFACE )
else()
find_package(googletest REQUIRED)
target_link_libraries(ext-gtest INTERFACE gtest gtest_main)
endif()
endif()

CPMAddPackage(NAME units
Expand Down
18 changes: 13 additions & 5 deletions arbor/cable_cell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,22 @@ struct cable_cell_impl {
// The decorations on the cell.
decor decorations;

// Discretization
std::optional<cv_policy> discretization_;

// The placeable label to lid_range map
dynamic_typed_map<constant_type<std::unordered_multimap<hash_type, lid_range>>::type> labeled_lid_ranges;

cable_cell_impl(const arb::morphology& m, const label_dict& labels, const decor& decorations):
cable_cell_impl(const arb::morphology& m, const label_dict& labels, const decor& decorations, const std::optional<cv_policy>& cvp):
provider(m, labels),
dictionary(labels),
decorations(decorations)
decorations(decorations),
discretization_{cvp}
{
init();
}

cable_cell_impl(): cable_cell_impl({},{},{}) {}
cable_cell_impl(): cable_cell_impl({}, {}, {}, {}) {}

cable_cell_impl(const cable_cell_impl& other) = default;

Expand Down Expand Up @@ -203,6 +207,10 @@ struct cable_cell_impl {
}
};

const std::optional<cv_policy>& cable_cell::discretization() const { return impl_->discretization_; }
void cable_cell::discretization(cv_policy cvp) { impl_->discretization_ = std::move(cvp); }


using impl_ptr = std::unique_ptr<cable_cell_impl, void (*)(cable_cell_impl*)>;
impl_ptr make_impl(cable_cell_impl* c) {
return impl_ptr(c, [](cable_cell_impl* p){delete p;});
Expand Down Expand Up @@ -232,8 +240,8 @@ void cable_cell_impl::init() {
}
}

cable_cell::cable_cell(const arb::morphology& m, const decor& decorations, const label_dict& dictionary):
impl_(make_impl(new cable_cell_impl(m, dictionary, decorations)))
cable_cell::cable_cell(const arb::morphology& m, const decor& decorations, const label_dict& dictionary, const std::optional<cv_policy>& cvp):
impl_(make_impl(new cable_cell_impl(m, dictionary, decorations, cvp)))
boeschf marked this conversation as resolved.
Show resolved Hide resolved
{}

cable_cell::cable_cell(): impl_(make_impl(new cable_cell_impl())) {}
Expand Down
8 changes: 0 additions & 8 deletions arbor/cable_cell_param.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,6 @@ std::vector<defaultable> cable_cell_parameter_set::serialize() const {
for (const auto& [name, mech]: reversal_potential_method) {
D.push_back(ion_reversal_potential_method{name, mech});
}

if (discretization) {
D.push_back(*discretization);
}

return D;
}

Expand Down Expand Up @@ -163,9 +158,6 @@ decor& decor::set_default(defaultable what) {
else if constexpr (std::is_same_v<ion_reversal_potential_method, T>) {
defaults_.reversal_potential_method[p.ion] = p.method;
}
else if constexpr (std::is_same_v<cv_policy, T>) {
defaults_.discretization = std::forward<cv_policy>(p);
}
else if constexpr (std::is_same_v<ion_diffusivity, T>) {
if (p.scale.type() != iexpr_type::scalar) throw cable_cell_error{"Default values cannot have a scale."};
auto s = p.scale.get_scalar();
Expand Down
Loading