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

Set BOOST_NO_EXCEPTIONS and BOOST_NO_RTTI for GCC and math in standal… #786

Merged
merged 19 commits into from
May 31, 2022

Conversation

jzmaddock
Copy link
Collaborator

…one mode.

Disable everything not needed in error_handling.hpp when no exceptions are available.

…one mode.

Disable everything not needed in error_handling.hpp when no exceptions are available.
@jzmaddock
Copy link
Collaborator Author

@ckormanyos : can you see if this fixes most of your standalone + no exceptions issues?

We can very nearly get rid of the all the string and iostream related includes in no-exception handling situations, they are now required from just one place:

std::string m(message ? message : "");

Unfortunately, there's no way we can tell at preprocessor time whether that procedure will be required. However, if it's not instantiated, none of the string or iostream code is ever used or instantiated.

@ckormanyos
Copy link
Member

Hi John (@jzmaddock) this does not entirely eliminate the problem(s).

I posted some sample code in the original post #785. If I play around, I can probably get some other tiny errors. But at the moment, i think Bernoulli might be throwing and is not caught in the expected mechanism.

@ckormanyos
Copy link
Member

Ahhh... Last but not least, this guy throws here.

Your fix is almost complete for the moment. i will check for some other throwing activities in the general code base.

@ckormanyos
Copy link
Member

It seems like we have a couple of rogue throw potentials --- also a handful in my own creation of cstdfloat.

How should we deal with these?

grafik

@jzmaddock
Copy link
Collaborator Author

I think I can see how to fix up bernoulli...

@ckormanyos
Copy link
Member

fix up bernoulli

Yes. Many thanks John. Commit a6db2d1 is confirmed to fix my test case.

{
if(n > m_capacity)
{
#ifndef BOOST_NO_EXCEPTIONS
Copy link
Member

Choose a reason for hiding this comment

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

I'm not an expert, but just curious here... Why is this unexpected event handled differently than the ones below that go through policies? Is there no such fitting policy for this action?

@ckormanyos
Copy link
Member

For the next trimesters or so, this is more than I had hoped for, upon running green...

Thank you John!

@ckormanyos
Copy link
Member

Hi John (@jzmaddock), based on this minute compiler phenomenon in my CI on an unrelated project on this branch, I took the liberty to add a tiny change.

Please give me your feedback on this at your convenience.

@ckormanyos
Copy link
Member

On another thought, and I'm not sure if I'd like to push for this right at this moment, but yet another CI job could ensure that spec-fun runs with -fno-exceptions and -fno-rtti on at least one of each modern GCC/clang.

I think spec-fun is where some of the exception-free clients will see some benefit, as these folks might (unknowingly) be on the metal or could eventually use such a feature to gain savings.

@jzmaddock
Copy link
Collaborator Author

On another thought, and I'm not sure if I'd like to push for this right at this moment, but yet another CI job could ensure that spec-fun runs with -fno-exceptions and -fno-rtti on at least one of each modern GCC/clang.

That was my thought too, unfortunately, running the tests locally shows that Boost.Test doesn't support -fno-exceptions so everything fails to build :(

@ckormanyos
Copy link
Member

Boost.Test doesn't support -fno-exceptions so everything fails to build

Thanks for looking into this John!

Ah yes, now that you mention it, that makes perfect sense.

I'm totally happy with the level reached so far and think if it's green, then it's good to go.

I consider us to be on a pathway toward reduced dependencies and flexibility. We might find a few things to improve (such in this PR here). Then we improve them. Maybe later, we find a few more. Maybe further down the road, we spend some time to build up a light-weight test environment, maybe not.

To re-iterate, I am really happy with thisw PR in its current state. I already ran my CI (doing a bunch of tgamma-like stuff) in another challenging, unrelated project on this branch. I ran with -fno-exceptions and -fno-rtti on GCC 4 through 11 and a bunch of clang versions and everything worked perfectly!

jzmaddock added 2 commits May 15, 2022 17:35
Add Ubuntu-22 plus gcc-12 and clang-14 CI tests.
Fix a couple headers which still had noeh-unfriendly code.
….com/boostorg/math into no_exceptions_or_rtti_error_handling

Fixed Conflicts:
	include/boost/math/special_functions/gamma.hpp
@ckormanyos
Copy link
Member

Wow! Even more awesomeness. Thanks John!

@ckormanyos
Copy link
Member

Hi John. I think one try-catch clause was missed in spec-fun no_eh in places like here.

The failures on Jammy look suspiciously like timeouts to me, although I'm not exactly sure what error code 143 means.

@ckormanyos
Copy link
Member

The problem in spec fun on no-eh is here, I believe.

I don't know what you'll find out on the failures on GCC. I have some very preliminary reports (from other projects) on what I might consider to be spurious behaviors in GCC12 on built-in memory operations, but that's super preliminary and not verified.

@jzmaddock
Copy link
Collaborator Author

Github is timing out whenever I try to access the logs at present, I'll take a look later...

@ckormanyos
Copy link
Member

Github is timing out whenever I try to access the logs at present, I'll take a look later

Yes. The pink unicorn is bucking and rearing its head

jzmaddock added 2 commits May 16, 2022 18:24
Doesn't easily support exception handling free compilation.
Otherwise we see the VM abort with out-of-memory errors.
@jzmaddock
Copy link
Collaborator Author

Still can't access the logs, so pushed a couple of speculative commits based on what I did manage to load!

@ckormanyos
Copy link
Member

ckormanyos commented May 17, 2022

Well, also this morning I'm still getting the pink Unicorn on log viewing.

Maybe it'll come to its senses later, but it's kind of hard to see what's going on without log viewing.

jzmaddock added 2 commits May 23, 2022 12:14
Reduces compiler memory footprint of those test cases.
@ckormanyos
Copy link
Member

Hi John, wonderful progress on this.

I reeally think this one is getting close. On this particular run, however, we get a hitch on no-eh here.

@jzmaddock
Copy link
Collaborator Author

I reeally think this one is getting close. On this particular run, however, we get a hitch on no-eh here.

Grrr, forgot to check those locally... at least this time we can actually see the logs! Old run cancelled and fix pushed.

@ckormanyos
Copy link
Member

This is cool.

It seems almost green. There does, however, seem to be an actual failure in a test case on Chatterjee in Drone on Ubuntu clang++-10 with language standard -std=gnu++17. It might be a tolerance, or the problem might be not yet known.

jzmaddock added 2 commits May 30, 2022 18:27
Increase tolerance in chatterjee_correlation test.
Remove a few tests from Github CI.
Remove autodiff from the sanitizer tests as they time out.
@jzmaddock
Copy link
Collaborator Author

It seems almost green. There does, however, seem to be an actual failure in a test case on Chatterjee in Drone on Ubuntu clang++-10 with language standard -std=gnu++17. It might be a tolerance, or the problem might be not yet known.

It's a tolerance thing: tolerance was 0.01 and the error found was 0.0103. I suspect @NAThompson and @mborland missed that one because we have one consistently failing test (an autodiff sanitizer test runs the machine out of memory I suspect), I've disabled that one, and also removed a few redundant Github tests - it doesn't cut down the testing load by much to be honest - I can't decide whether we should cull a few more or not - it feels like we should, but I worry about missing something...!

@ckormanyos
Copy link
Member

It's a tolerance thing: tolerance was 0.01 and the error found was 0.0103. I suspect @NAThompson and @mborland missed that one because we have one consistently failing test (an autodiff sanitizer test runs the machine out of memory I suspect), I've disabled that one, and also removed a few redundant Github tests - it doesn't cut down the testing load by much to be honest

Nice!

  • I can't decide whether we should cull a few more or not - it feels like we should, but I worry about missing something...!

Matt (@mborland) recently expressed interest in this topic.

One thing on GHA is that you can now selectively re-run either failed tests only or all tests entirely. I've used this selective re-run feature on Multiprecision successfully to go green. It does not cut down any testing time. Also the fail we mention here wasn't on GHA but Drone. Anyway, I thought I'd mention that if it was not known.

Thank you very much John (@jzmaddock) for driving onward in this PR!

@mborland
Copy link
Member

It's a tolerance thing: tolerance was 0.01 and the error found was 0.0103. I suspect @NAThompson and @mborland missed that one

It is safe to bump up the tolerance. It's shooting for correlation on random numbers compared to an approximate published value.

I can't decide whether we should cull a few more or not - it feels like we should, but I worry about missing something...!

Can we offload more testing to drone? It seems like those runs generally finish in an hour right now as opposed to the 6+ it takes GHA.

@jzmaddock jzmaddock merged commit 65aaf02 into develop May 31, 2022
@jzmaddock
Copy link
Collaborator Author

@mborland I'll look into shifting some load onto drone later.

@ckormanyos
Copy link
Member

ckormanyos commented May 31, 2022

Thank you John (@jzmaddock) for sticking with this mammoth PR. It entailed many remarkably detailed and difficult nuances.

I've already checked this branch out on the metal and in a bunch of unrelated CI tasks on other projects that dis-allow exception handling. It works fantastically for spec-fun-stuff.

If anything else arises, we can handle it in the next trimester. But this looks really great, light and clean for no-exception usage!

@NAThompson NAThompson deleted the no_exceptions_or_rtti_error_handling branch January 24, 2024 21:35
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