Generalize functions starting with s and t#1848
Conversation
…4.1 (tags/RELEASE_600/final)
|
@serban-nicusor-toptal I think Jenkins has some problems with windows workers here. |
|
Hmm, maybe its not limited to windows. |
|
Restart Stage sometimes fails to work, I restarted the entire job. |
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
# Conflicts: # stan/math/prim/fun/mdivide_left_ldlt.hpp
|
@SteveBronder @andrjohns This one is also waitinig for a review. |
andrjohns
left a comment
There was a problem hiding this comment.
Looks good, just some minor changes/clarifications
| * @param[in] A Matrix | ||
| * @param[in] B Matrix | ||
| * @param[in] t double | ||
| * @return exponential of At multiplies B |
There was a problem hiding this comment.
Similar to what? I don't understand.
There was a problem hiding this comment.
Sorry, I meant the 'At multiplies B' instead of 'At multiplied by B'
| * The transform is based on a centered stick-breaking process. | ||
| * | ||
| * @tparam T type of elements in the vector | ||
| * @tparam T type of the vector |
stan/math/prim/fun/size_mvt.hpp
Outdated
| * @throw std::invalid_argument since the type is a scalar. | ||
| */ | ||
| template <typename T> | ||
| template <typename T, require_stan_scalar_t<T>* = nullptr> |
There was a problem hiding this comment.
Should probably use more informative template params (e.g. ScalarT,EigT) for consistency with the rest of the functions
stan/math/prim/fun/to_array_1d.hpp
Outdated
| std::vector<T_val> result(matrix_size); | ||
| for (int i = 0; i < matrix_size; i++) { | ||
| result[i] = datap[i]; | ||
| result[i] = mat_ref.data()[i]; |
There was a problem hiding this comment.
What about something like:
std::vector<T_val> result(mat_ref.data(), mat_ref.data() + mat_ref.size());
There was a problem hiding this comment.
I just realized this wouldn't work for blocks (neither my code nor your suggestion)!
Added a test and fixed.
stan/math/rev/fun/sd.hpp
Outdated
| using T_vi = promote_scalar_t<vari*, T_map>; | ||
| using T_d = promote_scalar_t<double, T_map>; | ||
| vari** varis | ||
| = reinterpret_cast<vari**>(ChainableStack::instance_->memalloc_.alloc( |
There was a problem hiding this comment.
ChainableStack::instance_->memalloc_.alloc_array<vari*> is preferred now
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
Jenkins Console Log Machine informationProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010CPU: G++: Clang: |
Summary
Generalizes functions starting with s and t. All
trace_*_quad_formfunctions are also optimized by replacingtrace(multiply(A,B))withtranspose(A).cwiseProduct(B).sum().Tests
This is just a refactor. No new tests.
Side Effects
None.
Release notes
Generalized functions starting with s and t.
Checklist
Math issue Generalize matrix function signatures #1470
Copyright holder: Tadej Ciglarič
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