Fix csr_matrix_times_vector linker error #3048
Conversation
…_soa_sparse_matrix for making sparse matrices from csr formatted data
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: |
| sparse_var_value_t w_mat_arena | ||
| = to_soa_sparse_matrix<Eigen::RowMajor>(m, n, w, u_arena, v_arena); |
There was a problem hiding this comment.
Q: I assume bad things happen if you call to_soa_sparse_matrix and u and v aren't in the arena?
If so, could we just move the code which puts them in the area to inside that constructor? This file is basically the only place in the Math lib we currently put vectors of ints in the arena, as far as I can tell, but presumably any other CSR operations would want to do the same
There was a problem hiding this comment.
All of u, v, and w are copied over to the arena from within to_soa_sparse_matrix. An object already allocated on the arena that is handed off to to_arena() just returns the original object.
| for (int k = 0; k < w_mat_arena.adj().outerSize(); ++k) { | ||
| for (typename sparse_var_value_t::vari_type::InnerIterator it( |
There was a problem hiding this comment.
[optional] I find it a bit easier to read if we can keep the update_w function, just because it makes the link between the first case and this one a bit more obvious.
There was a problem hiding this comment.
Actually I found a much easier way to do this
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
Fixes stan-dev/rstanarm#620 and cleans up
csr_matrix_times_vector.The
csr_matrix_times_vectorfunction was erroring when linking. @WardBrian and I suspect the issue has something to do with the length of the mangled names of either the vtable elements or the windows debugger in mingw. Either way, this PR cuts down on the number of elements that need to be passed to the lamda in the reverse pass callback ofcsr_matrix_times_vectorwhich seems to remove the error. The full list of changes are below.csr_matrix_times_vector, we now always cast to a SoA Sparse Matrix before setting up the reverse pass callback.to_soa_sparse_matrixfor taking objects that are used to represent a sparse matrices CSR format and returning avar_value<SparseMatrix>.var_value<SparseMatrix/Matrix>that accepts two arena matrices for pre allocated values and adjoints. This is used into_soa_sparse_matrixso thatw's values and adjoints are only shallow copied into thevar_value<SparseMatrix>value_of()function for getting the values of aSparseMatrix<var>promote_scalar_typefor sparse matrix typesvari_value<SparseMatrix>(arena_matrix)constructor. If an arena matrix was passed in previously it would be shallow copied to the values and adjoints which would share the value pointer between the two. Now the outer index pointer and inner index pointer are shared between the two, but each has their own pointer for the values.Tests
Tests were added for the new
to_soa_matrix function. Tests for the changed functions can be run withRelease notes
Fixes linker issue with
csr_matrix_times_vectorChecklist
Copyright holder: Steve Bronder
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