stan-dev
Fix include orders to make sure specializations are used in classes
Description
We need to be more careful with our include orders when constructing classes. A good example is in the accumulator class where we call stan::math::sum(). On develop, the sum() being called there is actually the sum(std:vector<T>) in prim. The problem is that before a class is declared in C++ it needs to know all of the definitions of the functions it uses.
The fundamental problem is in the godbolt below, there it cannot find the double specialization for sum if it's included after the definition of accumulate.
https://godbolt.org/z/je89G573e
I'm still trying to think of a good solution to this. I think we need to include all of rev before including classes that do not have a rev specialization. Like for instance operands_and_partials is fine because it has a rev version. I think this may just be a bug with accumulator? If so then a solution like #2535 is a simple but non-DRY solution that just includes a rev specialization for accumulator. I need to check out how this effects functions that use classes that have a rev specialization.
Current Version:
v4.1.0
Steve, want to try to tackle this together in the upcoming week? We can try to pair code this; I know it’s tricky and I’ve used different techniques to solve some of the issues in the past. Maybe there’s something better we can come up with.
On Mon, Jul 19, 2021 at 2:03 PM Steve Bronder @.***> wrote:
Description
We need to be more careful with our include orders when constructing classes. A good example is in the accumulator https://github.com/stan-dev/math/blob/develop/stan/math/prim/fun/accumulator.hpp class where we call stan::math::sum(). On develop, the sum() being called there is actually the sum(std:vector<T>) in prim https://github.com/stan-dev/math/blob/develop/stan/math/prim/fun/sum.hpp. The problem is that before a class is declared in C++ it needs to know all of the definitions of the functions it uses.
The fundamental problem is in the godbolt below, there it cannot find the double specialization for sum if it's included after the definition of accumulate.
https://godbolt.org/z/je89G573e
I'm still trying to think of a good solution to this. I think we need to include all of rev before including classes that do not have a rev specialization. Like for instance operands_and_partials is fine because it has a rev version. I think this may just be a bug with accumulator? If so then a solution like #2535 https://github.com/stan-dev/math/pull/2535 is a simple but non-DRY solution that just includes a rev specialization for accumulator. I need to check out how this effects functions that use classes that have a rev specialization. Current Version:
v4.1.0
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/stan-dev/math/issues/2543, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADH6F4CA2WOBYYHL4LBO7DTYRSF5ANCNFSM5AUGB3IA .
