Skip to content

DPL / DPL Analysis: Introduce LabeledArray as an option for Variant #5313

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
ktf merged 2 commits into from
Jan 27, 2021
Merged

DPL / DPL Analysis: Introduce LabeledArray as an option for Variant #5313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b83f60e
Introduce LabeledArray as an option for Variant
aalkin Jan 14, 2021
67a2da2
add <cassert> include
aalkin Jan 27, 2021
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
2 changes: 2 additions & 0 deletions Analysis/Tutorials/src/configurableObjects.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ auto printMatrix(Array2D<T> const& m)
}

static constexpr float defaultm[3][4] = {{1.1, 1.2, 1.3, 1.4}, {2.1, 2.2, 2.3, 2.4}, {3.1, 3.2, 3.3, 3.4}};
static LabeledArray<float> la{&defaultm[0][0], 3, 4, {"r1", "r2", "r3"}, {"c1", "c2", "c3", "c4"}};

struct ConfigurableObjectDemo {
Configurable<configurableCut> cut{"cut", {0.5, 1, true}, "generic cut"};
Expand All @@ -60,6 +61,7 @@ struct ConfigurableObjectDemo {
// note that size is fixed by this declaration - externally supplied vector needs to be the same size!
Configurable<std::vector<int>> array{"array", {0, 0, 0, 0, 0, 0, 0}, "generic array"};
Configurable<Array2D<float>> vmatrix{"matrix", {&defaultm[0][0], 3, 4}, "generic matrix"};
Configurable<LabeledArray<float>> vla{"vla", {defaultm[0], 3, 4, {"r1", "r2", "r3"}, {"c1", "c2", "c3", "c4"}}, "labeled array"};

void init(InitContext const&){};
void process(aod::Collision const&, aod::Tracks const& tracks)
Expand Down
148 changes: 146 additions & 2 deletions Framework/Core/include/Framework/Array2D.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
#define FRAMEWORK_ARRAY2D_H
#include <cstdint>
#include <vector>
#include <unordered_map>
#include <memory>
#include <string>
#include <cassert>

namespace o2::framework
{
Expand All @@ -33,7 +37,7 @@ struct Array2D {
data = new T[rows * cols];
for (auto i = 0U; i < rows; ++i) {
for (auto j = 0U; j < cols; ++j) {
data[i * cols + j] = *(data_ + (i * cols + j));
data[i * cols + j] = data_[i * cols + j];
}
}
}
Expand All @@ -55,7 +59,7 @@ struct Array2D {
data = new T[rows * cols];
for (auto i = 0U; i < rows; ++i) {
for (auto j = 0U; j < cols; ++j) {
data[i * cols + j] = *(other.data + (i * cols + j));
data[i * cols + j] = other.data[i * cols + j];
}
}
}
Expand Down Expand Up @@ -114,6 +118,146 @@ struct Array2D {
uint32_t rows;
uint32_t cols;
};

static constexpr const char* const labels_rows_str = "labels_rows";
static constexpr const char* const labels_cols_str = "labels_cols";

template <typename T>
class LabeledArray
{
public:
using element_t = T;

LabeledArray()
: values{},
labels_rows{},
labels_cols{},
rowmap{},
colmap{}
{
}

LabeledArray(T const* data, uint32_t rows, uint32_t cols, std::vector<std::string> labels_rows_ = {}, std::vector<std::string> labels_cols_ = {})
: values{data, rows, cols},
labels_rows{labels_rows_},
labels_cols{labels_cols_},
rowmap{},
colmap{}
{
if (labels_rows.empty() == false) {
assert(labels_rows.size() == rows);
for (auto i = 0u; i < labels_rows.size(); ++i) {
rowmap.emplace(labels_rows[i], (uint32_t)i);
}
}
if (labels_cols.empty() == false) {
assert(labels_cols.size() == cols);
for (auto i = 0u; i < labels_cols.size(); ++i) {
colmap.emplace(labels_cols[i], (uint32_t)i);
}
}
}

LabeledArray(T const* data, uint32_t size, std::vector<std::string> labels_ = {})
: values{data, 1, size},
labels_rows{},
labels_cols{labels_},
rowmap{},
colmap{}
{
if (labels_cols.empty() == false) {
assert(labels_cols.size() == size);
for (auto i = 0u; i < labels_cols.size(); ++i) {
colmap.emplace(labels_cols[i], (uint32_t)i);
}
}
}

LabeledArray(Array2D<T> const& data, std::vector<std::string> labels_rows_ = {}, std::vector<std::string> labels_cols_ = {})
: values{data},
labels_rows{labels_rows_},
labels_cols{labels_cols_},
rowmap{},
colmap{}
{
if (labels_rows.empty() == false) {
assert(labels_rows.size() == values.rows);
for (auto i = 0u; i < labels_rows.size(); ++i) {
rowmap.emplace(labels_rows[i], (uint32_t)i);
}
}
if (labels_cols.empty() == false) {
assert(labels_cols.size() == values.cols);
for (auto i = 0u; i < labels_cols.size(); ++i) {
colmap.emplace(labels_cols[i], (uint32_t)i);
}
}
}

LabeledArray(LabeledArray<T> const& other) = default;
LabeledArray(LabeledArray<T>&& other) = default;
LabeledArray& operator=(LabeledArray<T> const& other) = default;
LabeledArray& operator=(LabeledArray<T>&& other) = default;

~LabeledArray() = default;

T get(uint32_t y, uint32_t x) const
{
return values(y, x);
}

T get(std::string y, std::string x) const
{
return values(rowmap.find(y)->second, colmap.find(x)->second);
}

T get(uint32_t x) const
{
return values[0][x];
}

T getRow(u_int32_t y) const
{
return values[y];
}

T* operator[](uint32_t y) const
{
return values[y];
}

auto getLabelsRows()
{
return labels_rows;
}

auto getLabelsCols()
{
return labels_cols;
}

auto getData()
{
return values;
}

auto rows()
{
return values.rows;
}

auto cols()
{
return values.cols;
}

private:
Array2D<T> values;
std::vector<std::string> labels_rows;
std::vector<std::string> labels_cols;
std::unordered_map<std::string, uint32_t> rowmap;
Copy link
Member

@ktf ktf Jan 27, 2021

Choose a reason for hiding this comment

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

do you really need the unordered_map?

Copy link
Member Author

@aalkin aalkin Jan 27, 2021

Choose a reason for hiding this comment

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

It was the easiest way to make label getter work without too much performance drop. I will try to rework this later.

Copy link
Member

@ktf ktf Jan 27, 2021

Choose a reason for hiding this comment

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

Did you check what was the actual performance issue? I strongly believe that in get(std::string, std::string) the most time consuming part is allocating the strings, not the lookup. Why not using get(char const*, char const*)?

Copy link
Member Author

@aalkin aalkin Jan 27, 2021

Choose a reason for hiding this comment

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

Getter arguments should've been references... The issue was that using std::find on string vector was slower than map lookup even for sizes below 10. And since typical usage would be getting non-sequential values of the array to use in multiple if statements in process() that could be a problem. But I do not like using unordered_map and intend to rewrite it with the hash trick we use in registries.

Copy link
Member

@ktf ktf Jan 27, 2021

Choose a reason for hiding this comment

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

even if they are references, if the common usage is get("foo", "bar"), as I assume it is, it will do two allocations for each invocation (because char const* is not a string). At least you should provide an overload, if that is a typical usecase.

std::unordered_map<std::string, uint32_t> colmap;
};
} // namespace o2::framework

#endif // FRAMEWORK_ARRAY2D_H
2 changes: 2 additions & 0 deletions Framework/Core/include/Framework/ConfigParamRegistry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ class ConfigParamRegistry
return vectorFromBranch<typename T::value_type>(mStore->store().get_child(key));
} else if constexpr (is_base_of_template<o2::framework::Array2D, T>::value) {
return array2DFromBranch<typename T::element_t>(mStore->store().get_child(key));
} else if constexpr (is_base_of_template<o2::framework::LabeledArray, T>::value) {
return labeledArrayFromBranch<typename T::element_t>(mStore->store().get_child(key));
} else if constexpr (std::is_same_v<T, boost::property_tree::ptree>) {
return mStore->store().get_child(key);
} else if constexpr (std::is_constructible_v<T, boost::property_tree::ptree>) {
Expand Down
4 changes: 3 additions & 1 deletion Framework/Core/include/Framework/ConfigParamSpec.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ struct ConfigParamSpec {

ConfigParamSpec(std::string _name, ParamType _type,
Variant _defaultValue, HelpString _help)
: name(_name), type(_type), defaultValue(_defaultValue), help(_help) {}
: name(_name), type(_type), defaultValue(_defaultValue), help(_help)
{
}

/// config spec without default value, explicitely marked as 'empty'
/// and will be ignored at other places
Expand Down
2 changes: 1 addition & 1 deletion Framework/Core/include/Framework/Configurable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace o2::framework
template <typename T>
struct ConfigurableBase {
ConfigurableBase(std::string const& name, T&& defaultValue, std::string const& help)
: name(name), value{std::move(defaultValue)}, help(help)
: name(name), value{std::forward<T>(defaultValue)}, help(help)
{
}
using type = T;
Expand Down
33 changes: 28 additions & 5 deletions Framework/Core/include/Framework/Variant.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ enum class VariantType : int { Int = 0,
Array2DInt,
Array2DFloat,
Array2DDouble,
LabeledArrayInt,
LabeledArrayFloat,
LabeledArrayDouble,
Empty,
Unknown };

Expand All @@ -54,6 +57,12 @@ constexpr auto isArray2D()
return (V == VariantType::Array2DInt || V == VariantType::Array2DFloat || V == VariantType::Array2DDouble);
}

template <VariantType V>
constexpr auto isLabeledArray()
{
return (V == VariantType::LabeledArrayInt || V == VariantType::LabeledArrayFloat || V == VariantType::LabeledArrayDouble);
}

template <typename T>
struct variant_trait : std::integral_constant<VariantType, VariantType::Unknown> {
};
Expand Down Expand Up @@ -93,6 +102,10 @@ DECLARE_VARIANT_TRAIT(Array2D<int>, Array2DInt);
DECLARE_VARIANT_TRAIT(Array2D<float>, Array2DFloat);
DECLARE_VARIANT_TRAIT(Array2D<double>, Array2DDouble);

DECLARE_VARIANT_TRAIT(LabeledArray<int>, LabeledArrayInt);
DECLARE_VARIANT_TRAIT(LabeledArray<float>, LabeledArrayFloat);
DECLARE_VARIANT_TRAIT(LabeledArray<double>, LabeledArrayDouble);

template <typename T>
struct variant_array_symbol {
constexpr static char symbol = 'u';
Expand Down Expand Up @@ -153,6 +166,10 @@ DECLARE_VARIANT_TYPE(Array2D<int>, Array2DInt);
DECLARE_VARIANT_TYPE(Array2D<float>, Array2DFloat);
DECLARE_VARIANT_TYPE(Array2D<double>, Array2DDouble);

DECLARE_VARIANT_TYPE(LabeledArray<int>, LabeledArrayInt);
DECLARE_VARIANT_TYPE(LabeledArray<float>, LabeledArrayFloat);
DECLARE_VARIANT_TYPE(LabeledArray<double>, LabeledArrayDouble);

template <VariantType type>
struct variant_array_element_type {
};
Expand All @@ -172,6 +189,13 @@ DECLARE_VARIANT_ARRAY_ELEMENT_TYPE(double, Array2DDouble);
DECLARE_VARIANT_ARRAY_ELEMENT_TYPE(bool, ArrayBool);
DECLARE_VARIANT_ARRAY_ELEMENT_TYPE(std::string, ArrayString);

DECLARE_VARIANT_ARRAY_ELEMENT_TYPE(int, LabeledArrayInt);
DECLARE_VARIANT_ARRAY_ELEMENT_TYPE(float, LabeledArrayFloat);
DECLARE_VARIANT_ARRAY_ELEMENT_TYPE(double, LabeledArrayDouble);

template <VariantType V>
using variant_array_element_type_t = typename variant_array_element_type<V>::type;

template <typename S, typename T>
struct variant_helper {
static void set(S* store, T value)
Expand All @@ -183,10 +207,6 @@ struct variant_helper {
{
*reinterpret_cast<T*>(store) = reinterpret_cast<T>(std::memcpy(std::malloc(size * sizeof(std::remove_pointer_t<T>)), reinterpret_cast<void*>(values), size * sizeof(std::remove_pointer_t<T>)));
}
static void reset(S* store, T values, size_t)
{
*reinterpret_cast<T*>(store) = values;
}

static T get(const S* store) { return *(reinterpret_cast<const T*>(store)); }
};
Expand Down Expand Up @@ -215,7 +235,10 @@ struct variant_helper<S, std::string> {
/// Variant for configuration parameter storage. Owns stored data.
class Variant
{
using storage_t = std::aligned_union<8, int, int64_t, const char*, float, double, bool, int*, float*, double*, bool*, Array2D<int>, Array2D<float>, Array2D<double>>::type;
using storage_t = std::aligned_union<8, int, int64_t, const char*, float, double, bool,
int*, float*, double*, bool*,
Array2D<int>, Array2D<float>, Array2D<double>,
LabeledArray<int>, LabeledArray<float>, LabeledArray<double>>::type;

public:
Variant(VariantType type = VariantType::Unknown) : mType{type}, mSize{1} {}
Expand Down
Loading