Skip to content

Math standalone #418

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

Merged
merged 22 commits into from
Feb 28, 2022
Merged

Math standalone #418

merged 22 commits into from
Feb 28, 2022

Conversation

mborland
Copy link
Member

Offer better context detection of Boost.Math standalone in conjunction with Boost.MP standalone.

@mborland
Copy link
Member Author

@jzmaddock Almost all of my failures are in the concepts test where fpclassify/isnan/isinf are replaced with a macro that is designed to generate a compilation failure (example). Is there a workaround for this or something I am missing? I am just calling the boost::math implementation for almost all of these (see new <boost/multiprecision/detail/fpclassify.hpp>)

@jzmaddock
Copy link
Collaborator

Although they're declining in number, there are some platforms that define isnan/isinf etc as macros (rather like the min/max issue on Windows), so all those functions need to be macro expansion proof.

For declarations we can do that with:

bool isnan BOOST_PREVENT_MACRO_SUBSTITUTION(T args); // or
bool isnan /*Suppress macro expansion*/(T args);

For calls it's either:

(boost::math::isnan)(x) // or
boost::math::isnan  BOOST_PREVENT_MACRO_SUBSTITUTION(x); // or
boost::math::isnan /*Suppress macro expansion*/(x);

HTH, John.

@mborland mborland mentioned this pull request Feb 8, 2022
@jzmaddock
Copy link
Collaborator

Hi Matt, thanks for persevering with this, I think we can simply things here quite a bit, but I need to check through the changes line by line to be sure, so be prepared for a small blizzard of line-comments!

@jzmaddock
Copy link
Collaborator

OK, apologies for the comment blizzard, but I think we can simplify things a lot with this.

Note that in general, I'd rather not replace calls to boost::math::isnan to std::isnan as the former keeps working even when certain "fast-and-loose" optimisations are in place. One option to reduce the #if logic, would be a macro that expands to either using std::isnan or using boost::math::isnan depending on which is available.

@mborland
Copy link
Member Author

I think this latest commit simplifies what you were looking for. It defines macros such as BOOST_MP_ISINF that either expands to boost::math::isinf if it is available, or SFINAEd definitions of isinf depending on if the type is supported by existing definitions (e.g. STL and libquadmath)

@jzmaddock
Copy link
Collaborator

Thanks Matt, can we just get rid of the floor/ceil code in place of std::floor/ceil, and then hopefully this should be good to go?

@mborland
Copy link
Member Author

Thanks Matt, can we just get rid of the floor/ceil code in place of std::floor/ceil, and then hopefully this should be good to go?

Locally I don't see any issues with std::floor/ceil so the superfluous code is gone now. Sorry I missed that one with the fixes earlier.

@mborland
Copy link
Member Author

@jzmaddock This should be good now. The 2 failures are timeouts on the CI's side.

@mborland
Copy link
Member Author

Upon a deeper look this is in fact not good to go. I spun up a VM that has nothing installed to see where we are at. There are more lingering instances that need to be removed/replaced.

@mborland
Copy link
Member Author

@jzmaddock This is now good to go. I pulled this branch into a VM with none of boost installed and it all compiles now.

@mariospr
Copy link
Contributor

mariospr commented Feb 24, 2022

FWIW, and just in case it helps: I was initially not being able to build this multiprecision library in standalone mode off the develop branch because of several references to <boost/math/...> not guarded by the BOOST_MP_STANDALONE define, when I tried to do it yesterday for the first time before finding this PR.

But then I found this PR and I can confirm I could build in standalone mode now with the only addition of Boost.Config, which I had to download as well as it's still referenced from standalone_config.hpp (despite of the comment saying that this package "already includes a copy of Boost.Config so no other donwloads are required", which I must say confused me a bit).

Anyway, this is not a review (I'm not qualified at all to do so), just a quick comment based on my recent experience in case it helps in some way. Thanks a lot for this PR in any case!.

@mborland
Copy link
Member Author

@mariospr I am glad to hear that this PR works for you. The intent is to have a packaged standalone release on the home page much like we do with Boost.Math that will include Boost.Config. The updates to the docs may have been pre-mature so I apologize for the confusion.

@mariospr
Copy link
Contributor

@mariospr I am glad to hear that this PR works for you. The intent is to have a packaged standalone release on the home page much like we do with Boost.Math that will include Boost.Config.

Ah, I see, that makes a lot of sense, thanks for clarifying.

The updates to the docs may have been pre-mature so I apologize for the confusion.

No worries. I'm not too familiar with Boost so I was unsure after reading the README whether I was missing something out, but this explains things, thanks again.

@mborland
Copy link
Member Author

Ping @jzmaddock. We are coming up on the beta/release window, and I think it's important this makes it.

@jzmaddock jzmaddock merged commit 7e8d2ae into boostorg:develop Feb 28, 2022
@mborland mborland deleted the math_standalone branch March 1, 2022 08:24
@mborland mborland mentioned this pull request Mar 1, 2022
mariospr added a commit to brave/brave-core that referenced this pull request Mar 1, 2022
The PR to complete the implementation of the standalone mode [1] has
been finally merged so we can point to the bootsorg's GH repository.

[1] boostorg/multiprecision#418
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.

3 participants