remove var_alloc_stack_ and dependencies

[bob-carpenter]

Summary:

Replace var_alloc_stack_ data structure and rewrite functions that depend on it

• [ ] replace all the functions that depend on var_alloc_stack_ with explicit allocations wrapped in Map
  • [ ] rev/mat/fun/LDLT_alloc.hpp
  • [ ] rev/mat/fun/mdivide_left_ldlt.hpp
  • [ ] rev/mat/fun/mdivide_left_spd.hpp
  • [x] rev/mat/fun/quad_form.hpp
  • [x] rev/mat/fun/quad_form_sym.hpp
  • [x] rev/mat/fun/trace_gen_quad_form.hpp
  • [ ] rev/mat/fun/trace_inv_quad_form_ldlt.hpp
  • [x] rev/mat/fun/trace_quad_form.hpp
  • [ ] rev/mat/fun/log_determinant_ldlt.hpp
• [ ] in rev/core/autodiffstackstorage.hppremove the member variable AutodiffStackStorage::var_alloc_stack_
• [ ] remove file rev/core/chainable_alloc.hpp
• [ ] add doc to the memory classes for reverse-mode autodiff
• [ ] remove calls made redundant in rev/core/recover_memory.hpp and rev/core/recover_memory_nested.hpp
• [ ] remove includes for removed removed files in
  • [ ] rev/core/grad.hpp
  • [ ] rev/core/set_zero_all_adjoints.hpp
  • [ ] rev/core/set_zero_all_adjoints_nested.hpp
  • [ ] rev/core.hpp

Description:

This will make the autodiff infrastructure simpler and a bit faster, because there'll only be two stacks to clean.

Current Version:

v2.16.0

[bbbales2]

Pull req. #927 (from branch feature/issue-565-vari-dealloc) removes the dependence of many of the varis that depended on chainable_alloc.

The issue with that pull request now is that in fixing these problems, it introduced some amount of complexity in the varis that changed that would not be necessary if they were re-implemented with adj_jac_apply.

The cleanup of LDLT_factor is worth keeping, and the cleanup and re-organization of tests is worthwhile as well.

For that pull request to be finished, what needs to happen is:

1. LDLT_factor currently has two constructors. One that takes and Eigen matrix and immediately computes the decomposition, and one that takes no arguments, and an Eigen matrix is passed to it later to be decomposed. The second should be removed so that the LDLT object always exists in good state (or has thrown an exception). This removes the need for any checks that the LDLT object is usable or not.

2. Remove stan/math/rev/core/build_double_array.hpp and stan/math/rev/core/build_vari_pointer_array_if_necessary.hpp from the pull request. This is the unnecessary complexity. Rewrite any of the functions that depend on them using adj_jac_apply (probably the simplest option) or other techniques. This may include functions in:

stan/math/rev/mat/fun/log_determinant_ldlt.hpp stan/math/rev/mat/fun/mdivide_left_ldlt.hpp stan/math/rev/mat/fun/mdivide_left_spd.hpp stan/math/rev/mat/fun/quad_form.hpp stan/math/rev/mat/fun/quad_form_sym.hpp stan/math/rev/mat/fun/trace_gen_inv_quad_form_ldlt.hpp stan/math/rev/mat/fun/trace_gen_quad_form.hpp stan/math/rev/mat/fun/trace_inv_quad_form_ldlt.hpp and stan/math/rev/mat/fun/trace_quad_form.hpp

[rok-cesnovar]

Is this issue still relevant? I am guessing with the recent refactors it is not?

[bbbales2]

I don't think this is gonna be feasible much longer or it's worth doing anymore. This is a useful feature. I think there's at least a thing in opencl that depends on this, the scoped chainable stack will depend on this, and we have this code computing a QR twice that could be avoided easily with another one of these https://github.com/stan-dev/math/blob/develop/stan/math/rev/fun/mdivide_left.hpp#L31 .