Skip to content

Commit

Permalink
Shuffle internal hash and combine into a detail namespace.
Browse files Browse the repository at this point in the history
  • Loading branch information
thorstenhater committed Nov 29, 2023
1 parent 0cec44f commit c583b2f
Show file tree
Hide file tree
Showing 12 changed files with 65 additions and 77 deletions.
5 changes: 2 additions & 3 deletions arbor/benchmark_cell_group.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#include <chrono>
#include <exception>

#include <arbor/arbexcept.hpp>
#include <arbor/benchmark_cell.hpp>
Expand Down Expand Up @@ -40,8 +39,8 @@ benchmark_cell_group::benchmark_cell_group(const std::vector<cell_gid_type>& gid
for (const auto& c: cells_) {
cg_sources.add_cell();
cg_targets.add_cell();
cg_sources.add_label(internal_hash(c.source), {0, 1});
cg_targets.add_label(internal_hash(c.target), {0, 1});
cg_sources.add_label(hash_value(c.source), {0, 1});
cg_targets.add_label(hash_value(c.target), {0, 1});
}

benchmark_cell_group::reset();
Expand Down
5 changes: 1 addition & 4 deletions arbor/cable_cell_param.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
#include <cfloat>
#include <cmath>
#include <memory>
#include <numeric>
#include <vector>
#include <variant>
#include <tuple>
Expand Down Expand Up @@ -122,7 +119,7 @@ decor& decor::paint(region where, paintable what) {
}

decor& decor::place(locset where, placeable what, cell_tag_type label) {
auto hash = internal_hash(label);
auto hash = hash_value(label);
if (hashes_.count(hash) && hashes_.at(hash) != label) {
throw arbor_internal_error{util::strprintf("Hash collision {} ./. {}", label, hashes_.at(hash))};
}
Expand Down
2 changes: 0 additions & 2 deletions arbor/include/arbor/cable_cell_param.hpp
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
#pragma once

#include <cmath>
#include <memory>
#include <optional>
#include <unordered_map>
#include <string>
#include <variant>
#include <any>

#include <arbor/export.hpp>
#include <arbor/arbexcept.hpp>
Expand Down
12 changes: 6 additions & 6 deletions arbor/include/arbor/util/hash_def.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@
#include <string_view>
#include <functional>

#include <iostream>

// Helpers for forming hash values of compounds objects.

namespace arb {

namespace detail {

// Non-cryptographic hash function for mapping strings to internal
// identifiers. Concretely, FNV-1a hash function taken from
//
Expand Down Expand Up @@ -87,10 +86,11 @@ std::size_t hash_value_combine(std::size_t n, const T& head, const Ts&... tail)
return hash_value_combine(prime*n + internal_hash(head), tail...);
}

template <typename... T>
std::size_t hash_value(const T&... ts) {
return hash_value_combine(0, ts...);
}

// User facing API
template <typename... T>
std::size_t hash_value(const T&... ts) { return detail::hash_value_combine(0, ts...); }
}

#define ARB_DEFINE_HASH(type,...)\
Expand Down
8 changes: 4 additions & 4 deletions arbor/label_resolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ cell_label_range::cell_label_range(std::vector<cell_size_type> size_vec,
{
std::transform(label_vec.begin(), label_vec.end(),
std::back_inserter(labels),
internal_hash<const std::string&>);
hash_value<const std::string&>);
arb_assert(check_invariant());
};

Expand Down Expand Up @@ -93,12 +93,12 @@ lid_hopefully label_resolution_map::range_set::at(unsigned idx) const {
}

const label_resolution_map::range_set& label_resolution_map::at(cell_gid_type gid, const cell_tag_type& tag) const {
return map.at(gid).at(internal_hash(tag));
return map.at(gid).at(hash_value(tag));
}

std::size_t label_resolution_map::count(cell_gid_type gid, const cell_tag_type& tag) const {
if (!map.count(gid)) return 0u;
return map.at(gid).count(internal_hash(tag));
return map.at(gid).count(hash_value(tag));
}

label_resolution_map::label_resolution_map(const cell_labels_and_gids& clg) {
Expand Down Expand Up @@ -216,7 +216,7 @@ lid_hopefully update_state(resolver::state_variant& v,
cell_lid_type resolver::resolve(cell_gid_type gid, const cell_local_label_type& label) {
const auto& [tag, pol] = label;

auto hash = internal_hash(tag);
auto hash = hash_value(tag);

if (!label_map_->count(gid, tag)) throw arb::bad_connection_label(gid, tag, "label does not exist");
const auto& range_set = label_map_->at(gid, tag);
Expand Down
3 changes: 1 addition & 2 deletions arbor/label_resolution.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <arbor/common_types.hpp>
#include <arbor/util/expected.hpp>

#include "util/partition.hpp"
#include <arbor/util/hash_def.hpp>

namespace arb {
Expand All @@ -26,7 +25,7 @@ struct ARB_ARBOR_API cell_label_range {
cell_label_range& operator=(const cell_label_range&) = default;
cell_label_range& operator=(cell_label_range&&) = default;

cell_label_range(std::vector<cell_size_type> size_vec, std::vector<cell_tag_type> label_vec, std::vector<lid_range> range_vec);
cell_label_range(std::vector<cell_size_type> size_vec, std::vector<cell_tag_type> label_vec, std::vector<lid_range> rapfnge_vec);
cell_label_range(std::vector<cell_size_type> size_vec, std::vector<hash_type> label_vec, std::vector<lid_range> range_vec);

void add_cell();
Expand Down
4 changes: 2 additions & 2 deletions arbor/lif_cell_group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ lif_cell_group::lif_cell_group(const std::vector<cell_gid_type>& gids,
// tell our caller about this cell's connections
cg_sources.add_cell();
cg_targets.add_cell();
cg_sources.add_label(internal_hash(cell.source), {0, 1});
cg_targets.add_label(internal_hash(cell.target), {0, 1});
cg_sources.add_label(hash_value(cell.source), {0, 1});
cg_targets.add_label(hash_value(cell.target), {0, 1});
// insert probes where needed
auto probes = rec.get_probes(gid);
for (const auto& probe: probes) {
Expand Down
4 changes: 1 addition & 3 deletions arbor/spike_source_cell_group.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#include <exception>

#include <arbor/arbexcept.hpp>
#include <arbor/recipe.hpp>
#include <arbor/spike_source_cell.hpp>
Expand Down Expand Up @@ -33,7 +31,7 @@ spike_source_cell_group::spike_source_cell_group(
try {
auto cell = util::any_cast<spike_source_cell>(rec.get_cell_description(gid));
time_sequences_.emplace_back(cell.seqs);
cg_sources.add_label(internal_hash(cell.source), {0, 1});
cg_sources.add_label(hash_value(cell.source), {0, 1});
}
catch (std::bad_any_cast& e) {
throw bad_cell_description(cell_kind::spike_source, gid);
Expand Down
27 changes: 14 additions & 13 deletions test/unit/test_cable_cell.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
#include <gtest/gtest.h>

#include "../common_cells.hpp"
#include <arbor/util/hash_def.hpp>

#include <arbor/util/hash_def.hpp>
#include <arbor/cable_cell.hpp>
#include <arbor/cable_cell_param.hpp>

Expand Down Expand Up @@ -46,20 +47,20 @@ TEST(cable_cell, lid_ranges) {
const auto& src_ranges = cell.detector_ranges();
const auto& tgt_ranges = cell.synapse_ranges();

EXPECT_EQ(1u, tgt_ranges.count(internal_hash("t0")));
EXPECT_EQ(1u, tgt_ranges.count(internal_hash("t1")));
EXPECT_EQ(1u, src_ranges.count(internal_hash("s0")));
EXPECT_EQ(1u, tgt_ranges.count(internal_hash("t2")));
EXPECT_EQ(1u, src_ranges.count(internal_hash("s1")));
EXPECT_EQ(2u, tgt_ranges.count(internal_hash("t3")));
EXPECT_EQ(1u, tgt_ranges.count(hash_value("t0")));
EXPECT_EQ(1u, tgt_ranges.count(hash_value("t1")));
EXPECT_EQ(1u, src_ranges.count(hash_value("s0")));
EXPECT_EQ(1u, tgt_ranges.count(hash_value("t2")));
EXPECT_EQ(1u, src_ranges.count(hash_value("s1")));
EXPECT_EQ(2u, tgt_ranges.count(hash_value("t3")));

auto r1 = tgt_ranges.equal_range(internal_hash("t0")).first->second;
auto r2 = tgt_ranges.equal_range(internal_hash("t1")).first->second;
auto r3 = src_ranges.equal_range(internal_hash("s0")).first->second;
auto r4 = tgt_ranges.equal_range(internal_hash("t2")).first->second;
auto r5 = src_ranges.equal_range(internal_hash("s1")).first->second;
auto r1 = tgt_ranges.equal_range(hash_value("t0")).first->second;
auto r2 = tgt_ranges.equal_range(hash_value("t1")).first->second;
auto r3 = src_ranges.equal_range(hash_value("s0")).first->second;
auto r4 = tgt_ranges.equal_range(hash_value("t2")).first->second;
auto r5 = src_ranges.equal_range(hash_value("s1")).first->second;

auto r6_range = tgt_ranges.equal_range(internal_hash("t3"));
auto r6_range = tgt_ranges.equal_range(hash_value("t3"));
auto r6_0 = r6_range.first;
auto r6_1 = std::next(r6_range.first);
if (r6_0->second.begin != 4u) {
Expand Down
44 changes: 20 additions & 24 deletions test/unit/test_fvm_lowered.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#include <cmath>
#include <numeric>
#include <string>
#include <vector>

Expand All @@ -25,11 +24,8 @@
#include "backends/multicore/fvm.hpp"
#include "fvm_lowered_cell.hpp"
#include "fvm_lowered_cell_impl.hpp"
#include "util/meta.hpp"
#include "util/maputil.hpp"
#include "util/rangeutil.hpp"
#include "util/span.hpp"
#include "util/transform.hpp"

#include "common.hpp"
#include "mech_private_field_access.hpp"
Expand Down Expand Up @@ -994,10 +990,10 @@ TEST(fvm_lowered, label_data) {
auto clg = cell_labels_and_gids(fvm_info.target_data, gids);
std::vector<cell_size_type> expected_sizes = {2, 0, 0, 2, 0, 0, 2, 0, 0, 2};
std::vector<std::pair<hash_type, lid_range>> expected_labeled_ranges = {
{internal_hash("1_synapse"), {4, 5}}, {internal_hash("4_synapses"), {0, 4}},
{internal_hash("1_synapse"), {4, 5}}, {internal_hash("4_synapses"), {0, 4}},
{internal_hash("1_synapse"), {4, 5}}, {internal_hash("4_synapses"), {0, 4}},
{internal_hash("1_synapse"), {4, 5}}, {internal_hash("4_synapses"), {0, 4}}
{hash_value("1_synapse"), {4, 5}}, {hash_value("4_synapses"), {0, 4}},
{hash_value("1_synapse"), {4, 5}}, {hash_value("4_synapses"), {0, 4}},
{hash_value("1_synapse"), {4, 5}}, {hash_value("4_synapses"), {0, 4}},
{hash_value("1_synapse"), {4, 5}}, {hash_value("4_synapses"), {0, 4}}
};

std::vector<std::pair<hash_type, lid_range>> actual_labeled_ranges;
Expand Down Expand Up @@ -1033,16 +1029,16 @@ TEST(fvm_lowered, label_data) {
auto clg = cell_labels_and_gids(fvm_info.source_data, gids);
std::vector<cell_size_type> expected_sizes = {1, 2, 2, 1, 2, 2, 1, 2, 2, 1};
std::vector<std::pair<hash_type, lid_range>> expected_labeled_ranges = {
{internal_hash("1_detector"), {0, 1}},
{internal_hash("2_detectors"), {3, 5}}, {internal_hash("3_detectors"), {0, 3}},
{internal_hash("2_detectors"), {3, 5}}, {internal_hash("3_detectors"), {0, 3}},
{internal_hash("1_detector"), {0, 1}},
{internal_hash("2_detectors"), {3, 5}}, {internal_hash("3_detectors"), {0, 3}},
{internal_hash("2_detectors"), {3, 5}}, {internal_hash("3_detectors"), {0, 3}},
{internal_hash("1_detector"), {0, 1}},
{internal_hash("2_detectors"), {3, 5}}, {internal_hash("3_detectors"), {0, 3}},
{internal_hash("2_detectors"), {3, 5}}, {internal_hash("3_detectors"), {0, 3}},
{internal_hash("1_detector"), {0, 1}}
{hash_value("1_detector"), {0, 1}},
{hash_value("2_detectors"), {3, 5}}, {hash_value("3_detectors"), {0, 3}},
{hash_value("2_detectors"), {3, 5}}, {hash_value("3_detectors"), {0, 3}},
{hash_value("1_detector"), {0, 1}},
{hash_value("2_detectors"), {3, 5}}, {hash_value("3_detectors"), {0, 3}},
{hash_value("2_detectors"), {3, 5}}, {hash_value("3_detectors"), {0, 3}},
{hash_value("1_detector"), {0, 1}},
{hash_value("2_detectors"), {3, 5}}, {hash_value("3_detectors"), {0, 3}},
{hash_value("2_detectors"), {3, 5}}, {hash_value("3_detectors"), {0, 3}},
{hash_value("1_detector"), {0, 1}}
};
std::vector<std::pair<hash_type, lid_range>> actual_labeled_ranges;

Expand Down Expand Up @@ -1077,12 +1073,12 @@ TEST(fvm_lowered, label_data) {
auto clg = cell_labels_and_gids(fvm_info.gap_junction_data, gids);
std::vector<cell_size_type> expected_sizes = {0, 2, 2, 0, 2, 2, 0, 2, 2, 0};
std::vector<std::pair<hash_type, lid_range>> expected_labeled_ranges = {
{internal_hash("1_gap_junction"), {2, 3}}, {internal_hash("2_gap_junctions"), {0, 2}},
{internal_hash("1_gap_junction"), {2, 3}}, {internal_hash("2_gap_junctions"), {0, 2}},
{internal_hash("1_gap_junction"), {2, 3}}, {internal_hash("2_gap_junctions"), {0, 2}},
{internal_hash("1_gap_junction"), {2, 3}}, {internal_hash("2_gap_junctions"), {0, 2}},
{internal_hash("1_gap_junction"), {2, 3}}, {internal_hash("2_gap_junctions"), {0, 2}},
{internal_hash("1_gap_junction"), {2, 3}}, {internal_hash("2_gap_junctions"), {0, 2}},
{hash_value("1_gap_junction"), {2, 3}}, {hash_value("2_gap_junctions"), {0, 2}},
{hash_value("1_gap_junction"), {2, 3}}, {hash_value("2_gap_junctions"), {0, 2}},
{hash_value("1_gap_junction"), {2, 3}}, {hash_value("2_gap_junctions"), {0, 2}},
{hash_value("1_gap_junction"), {2, 3}}, {hash_value("2_gap_junctions"), {0, 2}},
{hash_value("1_gap_junction"), {2, 3}}, {hash_value("2_gap_junctions"), {0, 2}},
{hash_value("1_gap_junction"), {2, 3}}, {hash_value("2_gap_junctions"), {0, 2}},
};

EXPECT_EQ(clg.gids, gids);
Expand Down
4 changes: 2 additions & 2 deletions test/unit/test_hash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

TEST(hash, string_eq) {
ASSERT_EQ(arb::hash_value("foobar"), arb::hash_value(std::string{"foobar"}));
ASSERT_EQ(arb::hash_value("foobar"), arb::internal_hash("foobar"));
ASSERT_NE(arb::hash_value("foobar"), arb::internal_hash("barfoo"));
ASSERT_EQ(arb::hash_value("foobar"), arb::hash_value("foobar"));
ASSERT_NE(arb::hash_value("foobar"), arb::hash_value("barfoo"));
}

TEST(hash, doesnt_compile) {
Expand Down
24 changes: 12 additions & 12 deletions test/unit/test_label_resolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ std::vector<hash_type> make_labels(const std::vector<std::string>& ls) {
std::vector<hash_type> res;
std::transform(ls.begin(), ls.end(),
std::back_inserter(res),
internal_hash<const std::string&>);
hash_value<const std::string&>);
return res;
}

Expand All @@ -23,7 +23,7 @@ TEST(test_cell_label_range, build) {

// Test add_cell and add_label
auto b0 = cell_label_range();
EXPECT_THROW(b0.add_label(internal_hash("l0"), {0u, 1u}), arb::arbor_internal_error);
EXPECT_THROW(b0.add_label(hash_value("l0"), {0u, 1u}), arb::arbor_internal_error);
EXPECT_TRUE(b0.sizes.empty());
EXPECT_TRUE(b0.labels.empty());
EXPECT_TRUE(b0.ranges.empty());
Expand All @@ -40,25 +40,25 @@ TEST(test_cell_label_range, build) {

auto b2 = cell_label_range();
b2.add_cell();
b2.add_label(internal_hash("l0"), {0u, 1u});
b2.add_label(internal_hash("l0"), {3u, 13u});
b2.add_label(internal_hash("l1"), {0u, 5u});
b2.add_label(hash_value("l0"), {0u, 1u});
b2.add_label(hash_value("l0"), {3u, 13u});
b2.add_label(hash_value("l1"), {0u, 5u});
b2.add_cell();
b2.add_cell();
b2.add_label(internal_hash("l2"), {6u, 8u});
b2.add_label(internal_hash("l3"), {1u, 0u});
b2.add_label(internal_hash("l4"), {7u, 2u});
b2.add_label(internal_hash("l4"), {7u, 2u});
b2.add_label(internal_hash("l2"), {7u, 2u});
b2.add_label(hash_value("l2"), {6u, 8u});
b2.add_label(hash_value("l3"), {1u, 0u});
b2.add_label(hash_value("l4"), {7u, 2u});
b2.add_label(hash_value("l4"), {7u, 2u});
b2.add_label(hash_value("l2"), {7u, 2u});
EXPECT_EQ((ivec{3u, 0u, 5u}), b2.sizes);
EXPECT_EQ(make_labels(svec{"l0", "l0", "l1", "l2", "l3", "l4", "l4", "l2"}), b2.labels);
EXPECT_EQ((lvec{{0u, 1u}, {3u, 13u}, {0u, 5u}, {6u, 8u}, {1u, 0u}, {7u, 2u}, {7u, 2u}, {7u, 2u}}), b2.ranges);
EXPECT_TRUE(b2.check_invariant());

auto b3 = cell_label_range();
b3.add_cell();
b3.add_label(internal_hash("r0"), {0u, 9u});
b3.add_label(internal_hash("r1"), {10u, 10u});
b3.add_label(hash_value("r0"), {0u, 9u});
b3.add_label(hash_value("r1"), {10u, 10u});
b3.add_cell();
EXPECT_EQ((ivec{2u, 0u}), b3.sizes);
EXPECT_EQ(make_labels
Expand Down

0 comments on commit c583b2f

Please sign in to comment.