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

Update geometric.hpp #1127

Merged
merged 3 commits into from
Apr 30, 2024
Merged

Update geometric.hpp #1127

merged 3 commits into from
Apr 30, 2024

Conversation

kenarab
Copy link
Contributor

@kenarab kenarab commented Apr 29, 2024

The proposal is derived from this issue

#1126

This line should be corrected
At /math/distributions/geometric.hpp at line 404

  return std::log(p); // success_fraction
      

Now the code is

  return log(p); // success_fraction
      

When building that library. Throws this error

In file included from /opt/homebrew/include/boost/math/distributions.hpp:26:
/opt/homebrew/include/boost/math/distributions/geometric.hpp:404:16: error: unexpected namespace name 'log': expected expression
        return log(p); // success_fraction
               ^
1 error generated.
make[2]: *** [CMakeFiles/PonisIqCoreNLV.dir/Unity/unity_0_cxx.cxx.o] Error 1
make[1]: *** [CMakeFiles/PonisIqCoreNLV.dir/all] Error 2
make: *** [all] Error 2

The proposal is derived from this issue
boostorg#1126
Copy link
Member

@mborland mborland left a comment

Choose a reason for hiding this comment

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

log should be found by ADL for any RealType. The correct fix would be adding using std::log; under this line: https://github.com/boostorg/math/blob/develop/include/boost/math/distributions/geometric.hpp#L386. I think it also needs a using std::exp;.

Implemented suggested change in
boostorg#1127 (review)
@kenarab kenarab requested a review from mborland April 29, 2024 20:12
Copy link

codecov bot commented Apr 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.71%. Comparing base (28a5ffb) to head (1303786).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1127      +/-   ##
===========================================
+ Coverage    93.69%   93.71%   +0.01%     
===========================================
  Files          770      770              
  Lines        61057    61057              
===========================================
+ Hits         57209    57221      +12     
+ Misses        3848     3836      -12     
Files Coverage Δ
include/boost/math/distributions/geometric.hpp 89.28% <ø> (ø)

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28a5ffb...1303786. Read the comment docs.

@mborland
Copy link
Member

macOS clang failures are being addressed elsewhere

@mborland mborland merged commit 1399ad8 into boostorg:develop Apr 30, 2024
63 of 69 checks passed
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.

2 participants