quantile: use nth_element, support Eigen vectors for multi-p inputs - #3358
quantile: use nth_element, support Eigen vectors for multi-p inputs#3358andrjohns wants to merge 8 commits into
nth_element, support Eigen vectors for multi-p inputs#3358Conversation
nth_element, support Eigen vectors for multi-p inputs
|
Upstream ci failures look legitimate, is that going to be addressed here or is there an upstream PR that has to go first? |
Ah tricky. It looks like the implementation in This PR changed that vectorised case so that the same type as the input vec is returned (i.e., Two options I can think of here:
No real preference either way (both small code changes), but the second one makes sure that upstream is always functional. Any pref on your end? |
|
Oh it looks like upstream we caught an issue with the return type |
|
I don’t mind doing a linked PR in Stan where there will be a temporary breakage |
|
Realised that the simple solution was to just set the return type to probs vec type, no other changes required upstream. I've also simplified the implementation a fair bit |
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
Summary
Since we have a c++17 requirement, we can use
std::nth_elementfor thequantilefunction with a singlepinstead of sorting the entire input. This PR also replaces thestd::min_element/std::max_elementwith Eigen'sminCoeff()&maxCoeff()to take advantage of any vectorisation.For the multi-
pcase, this also relaxes the requirement that anstd::vector<double>is used for the container ofpand that anstd::vector<double>is returned. Now the return type is the same as the type of the input vectorTests
Additional tests added for using
VectorXdandRowVectorXdfor the multi-pinputsSide Effects
N/A
Release notes
Used
std::nth_elementto reduce algorithmic complexity ofquantileimplementation, support using Eigen containers for multiplepChecklist
Copyright holder: Andrew Johnson
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit)make test-headers)make test-math-dependencies)make doxygen)make cpplint)the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested