From 26bb6a5adfa0f5190a8c0bc4da492ca88e727af9 Mon Sep 17 00:00:00 2001 From: "Peter J. Jones" Date: Fri, 4 Sep 2026 14:43:37 +0200 Subject: [PATCH 1/4] Add some notes about building with profiling info --- Makefile | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Makefile b/Makefile index 1cfe311..9d85972 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,12 @@ BUILD_DIR ?= build +# NOTE: +# +# To build profiling versions of the executable targets: +# +# make clean +# CXXFLAGS='-pg -O0' LDFLAGS='-pg' make + .PHONEY: all clean test all: $(BUILD_DIR)/compile_commands.json From 038265222042e772c90389a4672a8bf5eac88007 Mon Sep 17 00:00:00 2001 From: "Peter J. Jones" Date: Fri, 4 Sep 2026 15:07:48 +0200 Subject: [PATCH 2/4] spectra: Add a way to select which parquet file to read from --- bin/mzp-inspect.cpp | 9 ++++++--- include/mzpeak/index.h | 24 +++++++++++++++++++++++- src/index.cpp | 30 +++++++++++++++++++++++++++--- 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/bin/mzp-inspect.cpp b/bin/mzp-inspect.cpp index 9d244f7..9d592e1 100644 --- a/bin/mzp-inspect.cpp +++ b/bin/mzp-inspect.cpp @@ -147,9 +147,9 @@ int print_fmd_kv(MzPeak::Index& index, } /******************************************************************************/ -int dump_spectra(MzPeak::Index& index) +int dump_spectra(MzPeak::Index& index, MzPeak::Index::SpectraSource source) { - auto spectra = index.spectra(); + auto spectra = index.spectra(source); for (std::size_t spectrum_index : std::views::iota(0ul, spectra.size())) { const auto& spectrum = spectra[spectrum_index]; @@ -188,6 +188,7 @@ int main(int argc, char* argv[]) "Used with --fmdkv to print the value of the given key"); desc.add_options()("spectra", "Print all m/z and intensity values"); + desc.add_options()("peaks", "Like --spectra but read from spectra_peaks.parquet"); po::positional_options_description pops; pops.add("file", 1); @@ -225,7 +226,9 @@ int main(int argc, char* argv[]) return print_fmd_kv(index, vmap["fmdkv"].as(), key); } else if (vmap.count("spectra")) { - dump_spectra(index); + dump_spectra(index, MzPeak::Index::SpectraSource::Data); + } else if (vmap.count("peaks")) { + dump_spectra(index, MzPeak::Index::SpectraSource::Peaks); } else { std::println("WARN: no command given"); return 1; diff --git a/include/mzpeak/index.h b/include/mzpeak/index.h index 4da5370..ba20d7b 100644 --- a/include/mzpeak/index.h +++ b/include/mzpeak/index.h @@ -44,10 +44,32 @@ class Index { std::vector::const_iterator find_file(Schema::EntityType::Type, Schema::DataKind::Type) const; + /** + * Indicates which source to fetch spectra data from. + */ + enum class SpectraSource { + /// Use the `spectra_data.parquet` file which may contain profile + /// or centroid spectra data. + Data, + + /// Use the `spectra_peaks.parquet` file which is optional and may + /// not exist. + Peaks, + }; + + /** + * Returns `true` if the give source file exists. + */ + bool has_spectra(SpectraSource) const; + /** * Access the spectra in the file. + * + * Throws an exception if `SpectraSource::Peaks` is request and does + * not exist. Use the `has_spectra` function to check for a peaks + * source before call this function. */ - Spectra spectra() const; + Spectra spectra(SpectraSource source = SpectraSource::Data) const; /** * Access the low-level MzPeak Manager object. diff --git a/src/index.cpp b/src/index.cpp index e2ae27b..2e0fb76 100644 --- a/src/index.cpp +++ b/src/index.cpp @@ -7,6 +7,7 @@ directory of this repository. */ #include +#include #include "mzpeak/data/signals.h" #include "mzpeak/exception.h" @@ -17,6 +18,21 @@ directory of this repository. namespace MzPeak { +/******************************************************************************/ +std::string source_to_file_name(Index::SpectraSource source) +{ + using enum Index::SpectraSource; + + switch (source) { + case Data: + return "spectra_data.parquet"; + case Peaks: + return "spectra_peaks.parquet"; + } + + std::unreachable(); +} + /******************************************************************************/ Index::Index(std::unique_ptr archive) : manager_(std::make_shared(std::move(archive))) @@ -34,12 +50,20 @@ Index::find_file(Schema::EntityType::Type et, Schema::DataKind::Type dk) const } /******************************************************************************/ -Spectra Index::spectra() const +bool Index::has_spectra(SpectraSource source) const +{ + auto data_it = manager_->find_file(source_to_file_name(source)); + return data_it != manager_->files().end(); +} + +/******************************************************************************/ +Spectra Index::spectra(SpectraSource source) const { - auto data_it = manager_->find_file("spectra_data.parquet"); + std::string file_name(source_to_file_name(source)); + auto data_it = manager_->find_file(file_name); if (data_it == manager_->files().end()) { - throw InvalidFormatError("missing files: spectra_data.parquet"); + throw InvalidFormatError("missing file spectra file: " + file_name); } std::unique_ptr data = From f794a277048691a5d4f1ee345bc3424735201100 Mon Sep 17 00:00:00 2001 From: "Peter J. Jones" Date: Fri, 4 Sep 2026 16:14:38 +0200 Subject: [PATCH 3/4] spectra: Only compute the default dimensions once --- include/mzpeak/spectra.h | 1 + src/spectra.cpp | 40 ++++++++++++++++++++++++---------------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/include/mzpeak/spectra.h b/include/mzpeak/spectra.h index 231ceba..7e8fa8a 100644 --- a/include/mzpeak/spectra.h +++ b/include/mzpeak/spectra.h @@ -34,6 +34,7 @@ class Spectra final : public Util::EnumerableProxy { // Internal data access. std::shared_ptr data_; std::shared_ptr manager_; + std::vector default_dims_; // Function to fetch a specific spectrum. Spectrum fetch(uint64_t); diff --git a/src/spectra.cpp b/src/spectra.cpp index 1cde1dc..08eee03 100644 --- a/src/spectra.cpp +++ b/src/spectra.cpp @@ -16,23 +16,11 @@ top-level directory of this repository. namespace MzPeak { /******************************************************************************/ -Spectra::Spectra(std::unique_ptr data, - std::shared_ptr manager) - : EnumerableProxy( - 0, std::bind(std::mem_fn(&Spectra::fetch), this, std::placeholders::_1)) - , data_(std::move(data)) - , manager_(std::move(manager)) -{ - // Update the record count. - resize(data_->record_count()); -} - -/******************************************************************************/ -Spectrum Spectra::fetch(uint64_t index) +std::vector initial_dims(const Data::Signals& data) { // These are the dimensions we'll project by default. std::vector dims = - data_->array_index()->dimensions() | std::views::filter([](auto& d) { + data.array_index()->dimensions() | std::views::filter([](auto& d) { return d.array_type == Schema::PSI::ArrayType::Mz || d.array_type == Schema::PSI::ArrayType::Intensity; }) | @@ -49,9 +37,29 @@ Spectrum Spectra::fetch(uint64_t index) }); dims.erase(to_erase.begin(), to_erase.end()); + return dims; +} + +/******************************************************************************/ +Spectra::Spectra(std::unique_ptr data, + std::shared_ptr manager) + : EnumerableProxy( + 0, std::bind(std::mem_fn(&Spectra::fetch), this, std::placeholders::_1)) + , data_(std::move(data)) + , manager_(std::move(manager)) + , default_dims_(initial_dims(*data_)) +{ + // Update the record count. + resize(data_->record_count()); +} + +/******************************************************************************/ +Spectrum Spectra::fetch(uint64_t index) +{ + std::unique_ptr slice = + data_->select(default_dims_, data_->index().eq(index)); - std::unique_ptr slice = data_->select(dims, data_->index().eq(index)); - return Spectrum(index, manager_, data_, dims, std::move(slice)); + return Spectrum(index, manager_, data_, default_dims_, std::move(slice)); } } // namespace MzPeak From b2895a348924d96e73b90fad4f88404854e3274e Mon Sep 17 00:00:00 2001 From: "Peter J. Jones" Date: Fri, 4 Sep 2026 16:41:41 +0200 Subject: [PATCH 4/4] Make EnumerableProxy easier to use, and remove the last use of std::function --- include/mzpeak/util/enumerable_proxy.h | 36 +++++++++++++++----------- src/spectra.cpp | 5 +--- test/util_test.cpp | 28 ++++++++++++++------ 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/include/mzpeak/util/enumerable_proxy.h b/include/mzpeak/util/enumerable_proxy.h index d8beee2..09a7116 100644 --- a/include/mzpeak/util/enumerable_proxy.h +++ b/include/mzpeak/util/enumerable_proxy.h @@ -10,7 +10,7 @@ directory of this repository. #include #include -#include +#include #include #include #include @@ -22,9 +22,6 @@ namespace MzPeak::Util { template class EnumerableProxy : public std::ranges::view_interface> { public: - /// A function that can fetch the requested value. - using fetch_t = std::function; - /// The iterator type. class Iterator { public: @@ -32,9 +29,9 @@ class EnumerableProxy : public std::ranges::view_interface> { using value_type = V; /// Constructor for a valid iterator. - Iterator(std::size_t n, fetch_t fetch) + Iterator(std::size_t n, EnumerableProxy* owner) : n_(n) - , fetch_(fetch) + , owner_(owner) , cache_() { } @@ -42,7 +39,7 @@ class EnumerableProxy : public std::ranges::view_interface> { /// Constructor for an invalid iterator. Iterator(std::size_t n) : n_(n) - , fetch_(nullptr) + , owner_(nullptr) , cache_() { } @@ -50,6 +47,8 @@ class EnumerableProxy : public std::ranges::view_interface> { /// Default construction also invalid; Iterator() : n_(0) + , owner_(nullptr) + , cache_() { } @@ -97,8 +96,9 @@ class EnumerableProxy : public std::ranges::view_interface> { { if (cache_.has_value() && cache_->first == n_) { return cache_->second; - } else if (fetch_ != nullptr) { - const_cast(this)->cache_ = std::make_pair<>(n_, fetch_(n_)); + } else if (owner_ != nullptr) { + auto* self = const_cast(this); + self->cache_ = std::make_pair<>(n_, self->owner_->fetch(n_)); return cache_->second; } else { throw InvalidIteratorError("attempt to dereference an invalid iterator"); @@ -107,7 +107,7 @@ class EnumerableProxy : public std::ranges::view_interface> { private: std::size_t n_; - fetch_t fetch_; + EnumerableProxy* owner_; std::optional> cache_; }; @@ -124,9 +124,8 @@ class EnumerableProxy : public std::ranges::view_interface> { ~EnumerableProxy() = default; /// Meaningful constructor. - EnumerableProxy(std::size_t count, fetch_t fetch) + EnumerableProxy(std::size_t count) : count_(count) - , fetch_(fetch) { } @@ -134,21 +133,28 @@ class EnumerableProxy : public std::ranges::view_interface> { std::size_t size() const { return count_; } /// Iterator to the first element. - Iterator begin() const { return Iterator(0, fetch_); } + Iterator begin() const + { + return Iterator(0, const_cast*>(this)); + } /// Iterator/sentinel representing one element beyond the last. Iterator end() const { return Iterator(count_); } // Access an element by its index. - V operator[](std::size_t n) { return fetch_(n); } + V operator[](std::size_t n) { return fetch(n); } protected: + friend class Iterator; + /// Update the internal count of records. void resize(std::size_t n) { count_ = n; } + /// The function to call to get a specific element. + virtual V fetch(uint64_t) = 0; + private: std::size_t count_ = 0; - fetch_t fetch_ = nullptr; }; } // namespace MzPeak::Util diff --git a/src/spectra.cpp b/src/spectra.cpp index 08eee03..3cad06d 100644 --- a/src/spectra.cpp +++ b/src/spectra.cpp @@ -43,14 +43,11 @@ std::vector initial_dims(const Data::Signals& data) /******************************************************************************/ Spectra::Spectra(std::unique_ptr data, std::shared_ptr manager) - : EnumerableProxy( - 0, std::bind(std::mem_fn(&Spectra::fetch), this, std::placeholders::_1)) + : EnumerableProxy(data->record_count()) , data_(std::move(data)) , manager_(std::move(manager)) , default_dims_(initial_dims(*data_)) { - // Update the record count. - resize(data_->record_count()); } /******************************************************************************/ diff --git a/test/util_test.cpp b/test/util_test.cpp index 0448a45..4d90ae1 100644 --- a/test/util_test.cpp +++ b/test/util_test.cpp @@ -13,18 +13,30 @@ directory of this repository. #include "mzpeak/util/enumerable_proxy.h" +namespace { +struct EPTest : public MzPeak::Util::EnumerableProxy { + EPTest() + : EnumerableProxy(0) + { + resize(v.size()); + } + + std::vector v{0, 1, 2, 3, 4, 5}; + int fetch(uint64_t i) { return v[i]; } +}; +} // namespace + /******************************************************************************/ BOOST_AUTO_TEST_CASE(enumerable_proxy_simple) { - std::vector v1{0, 1, 2, 3, 4, 5}, v2; - v2.reserve(v1.size()); - - MzPeak::Util::EnumerableProxy ep(v1.size(), - [v1](std::size_t n) { return v1[n]; }); + EPTest ep_test; + std::vector v; + v.reserve(ep_test.v.size()); - for (auto i : ep) { - v2.push_back(i); + for (auto i : ep_test) { + v.push_back(i); } - BOOST_TEST(v1 == v2); + BOOST_TEST(ep_test.v.size() == v.size()); + BOOST_TEST(ep_test.v == v); }