Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix usages of fabs in check_symmetric with llvm17 #2991

Merged
merged 2 commits into from
Dec 22, 2023
Merged

Conversation

WardBrian
Copy link
Member

Summary

This fixes an issue found in llvm17 (really libc++ 17, clang version seems to not be the important factor) which broke argument-dependent-lookup of the fabs function.
So far as I can tell, the only place we were depending on this that could actually get std::complex inputs is in check_symmetric.

To be completely honest, I'm not sure why this specific change fixes the issue, or if there is something better.

Reminder: we probably should not be using std::complex in Stan Math!

Tests

I added some tests, but because this is in a bleeding edge compiler they won't make much of a difference immediately. They will be run in the next bleeding edge pipeline run.

Side Effects

I don't believe so

Release notes

Fixed a compilation issue with the check_symmetric function on complex-valued inputs under LLVM 17

Checklist

  • Copyright holder: Simons Foundation

    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

    • unit tests pass (to run, use: ./runTests.py test/unit)
    • header checks pass, (make test-headers)
    • dependencies checks pass, (make test-math-dependencies)
    • docs build, (make doxygen)
    • code passes the built in C++ standards checks (make cpplint)
  • the code is written in idiomatic C++ and changes are documented in the doxygen

  • the new changes are tested

@WardBrian WardBrian linked an issue Dec 20, 2023 that may be closed by this pull request
Copy link
Collaborator

@andrjohns andrjohns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@WardBrian WardBrian merged commit ae90c3e into develop Dec 22, 2023
8 checks passed
@WardBrian WardBrian deleted the fix/2989-fabs-adl branch December 22, 2023 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compilation failure with Clang 17 and check_symmetric
3 participants