feat: add pick command for reduce - #429
Draft
henryiii wants to merge 6 commits into
Draft
Conversation
Add a pick command to algorithm::reduce, which selects an arbitrary subset of bins by index from an axis which is not ordered, like the category axis. Unlike slice, the picked bins do not have to be adjacent, and they may be given in any order, which reorders the bins in the new axis. Counts in bins which are not picked are added to the overflow bin, if it is present, consistent with how slice treats unordered axes; otherwise they are discarded. Axes opt into picking with a new special constructor which accepts the original axis and a vector of bin indices to keep. The new trait axis::traits::is_pickable detects this constructor, mirroring is_reducible. axis::category implements it; ordered axes throw invalid_argument, since picking a non-adjacent subset from them would require changing the axis type, which reduce does not support yet. Fixes boostorg#275 Assisted-by: ClaudeCode:claude-fable-5
Windows CI failed two ways: - C1128 (too many sections) when compiling algorithm_reduce_test.cpp; add /bigobj for the test under CMake, matching the b2 build which already sets it globally and the existing fill/operators tests. - C4702 (unreachable code, treated as error) for the non-pickable static_if branch in reduce(). Unlike is_reducible (true for all standard axes), is_pickable is false for most axes, so that branch is codegen'd and MSVC flags the value after the noreturn throw. Guard the function with a 4702 pragma, mirroring detail/fill.hpp. Assisted-by: ClaudeCode:claude-opus-4.8
The is_pickable trait matched any axis whose generic (iterable, metadata) constructor accepts a vector of indices, e.g. variable<double, std::vector<int>>, which broke compilation of reduce() with any command on such axes. The pick constructor now takes an axis::pick_tag so only axes that opt in match the trait. Also mirror the MSVC /bigobj flag in test/Jamfile, simplify the pick branch of the fill loop, hoist index validation out of the static_if, and copy metadata via metadata_base(src) like the shrink constructors. Assisted-by: ClaudeCode:claude-fable-5
henryiii
marked this pull request as draft
July 23, 2026 13:15
Replace the per-cell linear search over the picked indices with a lookup table built once per axis. Picking 1000 of 2000 categories in a 2000x500 histogram drops from ~200 ms to ~15 ms, on par with slice. The table also routes the underflow bin of a hypothetical pickable axis to the overflow bin instead of reading out of bounds; the Axis concept now documents this. Also move the MSVC pragma block above the doc comment and clarify in the guide that pick cannot be combined with other commands. Assisted-by: ClaudeCode:claude-fable-5
pick accepts an optional slice_mode like slice. In crop mode the counts of unpicked bins and of the original overflow bin are discarded instead of being moved to the overflow bin. Assisted-by: ClaudeCode:claude-fable-5
…tructor The pick constructor was the only user-facing signature in the library that requires std::vector. A pointer range matches the iterator-pair idiom of the other axis constructors and the primitive arguments of the reduce constructor, and does not freeze a container type into the Axis concept. Assisted-by: ClaudeCode:claude-fable-5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This was more complicated than I thought. I'll update the description later and look it over once more before undrafting again, and note all the decisions, and check to see what boost-histogram looks like downstream with this. Assuming there's no rush on features, as the next feature deadline is months away.
🤖 AI text below 🤖
Closes #275.
This adds a
pickcommand toalgorithm::reduce, which selects an arbitrary subset of bins by index from an axis which is not ordered, like the category axis. Unlikeslice, the bins do not have to be adjacent, and the new axis keeps them in the order given.Summary:
A(const A&, axis::pick_tag, const std::vector<index_type>&), detected by the newaxis::traits::is_pickabletrait. Thepick_tagdisambiguates the constructor from the generic(iterable, metadata)axis constructors, which could otherwise match by accident and break compilation ofreducefor unrelated axes. Onlycategoryopts in.pickcannot be combined with another reduce command on the same axis; indices must be unique and in range.