I think this might be a bit clearer if the naming wasReturnSparse orReturnDense (or something similar), so that it's explicit that the flag will affect the return type

Sorry I need to update the docs and this PRs summary. After discussion with@bob-carpenter,@nhuurre, and the Eigen dev team (mostly in#2597) I think it makes more sense to return a sparse matrix with all entries filled in. I do kind of wonder if we should have something like exp_nonzero(sparse_matrix) available at the Stan language level where it will only run over the nonzero entries. But we can sort that out at a later time

Doc type name doesn't match function declaration.

Also, neat trick!

The doc for both overloads state that a sparse matrix is returned, so might need to clarify which is which

Rather than iterating over each element, would it be more performant to Map the values as a vector and call the vectorised implementation? For example:

Map<Eigen::VectorXd>mapSparse(x.valuePtr(), x.nonZeros());apply_scalar_unary<F, Eigen::VectorXd>::apply(mapSparse);

It would def be better, though I avoided it because I was a bit confused about the compressed sparse matrix scheme Eigen uses. If you look at their docshere under 'The SparseMatrix class' they sometimes allow empty values so its easier to write to the sparse matrix like

Values:227_3514__1_178InnerIndices:12_024__2_14

I was worried that directly grabbing the values could lead to some bad things since there may be places in the value pointer that are not initialized. Another issue is that ourarena_matrix scheme assumes that if you are passing it an Eigen map that the memory has already been allocated elsewhere for use in the reverse mode pass, which may not explicitly be true for sparse matrices.

So I agree what your saying here is def faster, but imo for this first pass I'd rather do the safer path atm. Once we have everything working at the language level and know more about our constraints and assumptions we can make then we can come back and do these kinds of optimizations.

Hmm, okay now I'm thinking about this more and I think that at the stan language level we can make it so that you can only assign to elements that are already non-zero. So I wonder if what we can do here is just assume that we are always working with compressed matrices, since then we don't have to worry about the uninitialized values and can do the fast thing safely

Wouldn't we also expect sparse matrices to be detected as a container though?

So I'm kind of confused. It seems like a lot of the time we are declaring things containers when they are "vectorizable" so for sparse matrix I was like, "Well these are not really vectorizable so I would not say they are a container". imo I kind of feel like we should just havecontainer simply mean a container pointing to items containing dynamically allocated memory likestd::vector<std::vector<Eigen::Matrix>>. Like there is a few types of memory patterns that I'm thinking of here

std::vector<double/var> & Eigen::Matrix<double/var, R, C> where generic memory can be mapped to access linearly
std::vector<std::vector<double/var> & Eigen::Matrix<double/var, R, C>> where generic memory the container points to can be mapped to linearly
Eigen::SparseMatrix<double/var> If compressed, this can be mapped to generic memory and accessed linearly.

Hmm, maybe this PR is the wrong path and I should be using theapply_vector_unary pattern? Because then I would do what you are saying in another comment above and just calling the vectorized function over the memory