Repository navigation
Conversation
check_symmetric compared the off-diagonal entries with value_of, which leaves std::complex<var> for std::complex<fvar<var>>, and fabs of that does not compile (libstdc++'s std::abs reads __rep). Compare with value_of_rec instead; for fvar<var> this also stops the check from recording nodes on the tape. The difference is now a plain double or std::complex<double>, so call stan::math::abs instead of fabs behind two using-declarations. eigenvectors_sym (prim and OpenCL) passed "eigenvalues_sym" as the function name for its error messages.
Contributor
Author
|
Actually, I'm closing this, I think the bugs are too severe to go silently, which would happen with this fix. Better crash the compilation than produce garbage. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3477, see details there. I also adjusted wrong error messages, that were likely the result of a careless copypaste.
Important note: The decomposition is actually buggy for general complex-valued matrices anyway, this just makes it compile in the first place!! I will resolve the bugs in a separate PR.
Done with help of claude.
Tests
Added new tests that cover the autodiff for complex
eigendecompose_sym,eigenvalues_symandeigenvectors_sym.This problem mostly went unnoticed simply because the tests weren't there.
Side Effects
None.
Release notes
Fixed eigen decomposition failing to compile for autodiff of complex matrices.
Checklist
Copyright holder: Jáchym Barvínek jachymb@gmail.com
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