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

Add g++-5 to ci #425

Merged
merged 14 commits into from
Feb 2, 2022
Merged

Add g++-5 to ci #425

merged 14 commits into from
Feb 2, 2022

Conversation

ckormanyos
Copy link
Member

No description provided.

@ckormanyos
Copy link
Member Author

This first naive try to scope out GCC 5 on bionic may not work because not all the standards of the matrix are available.

@ckormanyos
Copy link
Member Author

The first naive try is probably destined to fail since GCC 5 does supports neither -std=c++14 nor -std=c++17 in the test matrix.

@ckormanyos
Copy link
Member Author

ckormanyos commented Feb 1, 2022

So this ran surprisingly well --- not perfect, but well. I think all the dedicated, consistent work on going standalone really paid off.

So... anyway, In a dedicated run, I added the basic Multiprecision g++-5 tests as shown here.

I had expected quite a few failures, but there was one single test failure in cpp_int_tests regarding a floating-point-exception. I can probably track down the root cause locally.

I don't know how to guage the excitement on getting g++-5 back running in CI. This can definitely wait for 1.80 (post 1.79), since there is a lot going on.

But it does, in fact, seem feasible to go for g++-5.

Thoughts?

Cc: @jzmaddock and @mborland and @NAThompson

@jzmaddock
Copy link
Collaborator

If it's straightforward then by all means go for it.

@ckormanyos
Copy link
Member Author

If it's straightforward then by all means go for it.

While underway, I found a TODO for C++11.

I took a try at reworking the timer facility based on a TODO note I encountered. I opted for a C++11 design which results in both a stopwatch facility as well as the original interface to a class called timer. Both of these may be useful.

I am a bit unsure of the new design I made.

If anyone gets a chance (no hurry though), ... could you please taka a look at this? (Because <chrono> tends to be a bit of a wily one somethines...).

Cc: @jzmaddock and @mborland

@ckormanyos
Copy link
Member Author

ckormanyos commented Feb 2, 2022

I think I found something. This might turn into a longer story. So I'll get this communication started. If I'm wrong please correct this.

I found that test_cpp_rational.cpp was generally running slowly (on PC locally) and also crashing in CI on g++-5 in GHA.

So I found this line. The numeric_limits::digits will give zero when compiling with __GNUC__ and using -std=c++11 (or 14, 17, etc.). So the number of random digits requested underflows and explodes.

Please note GHA tests are using something like -std=c++11 and not a GNU extension.

So in this PR, I have:

  • Repaired test_cpp_rational.cpp. Use <random> and generally C++11-ify it.
  • Completely reworked the timer facility to be C++11-ified.
  • Add g++-5 tests to CI's GHA jobs.

I am not getting through the traffic jamon CI.

But I believe the underflow on digits I mention above might actually be a real factor that we might want to address for 1.79 itself.

@jzmaddock and @mborland please do comment on this or correct my thinking if I'm off on a tangent here...

Many thanks, guys.

@jzmaddock
Copy link
Collaborator

So I found this line. The numeric_limits::digits will give zero when compiling with GNUC and using -std=c++11 (or 14, 17, etc.). So the number of random digits requested underflows and explodes.

Ah :( Yes, double_limb_type is __int128 and numeric_limits is not specialized unless you compile with --gnu++XX, I'll check over the diff shortly, this is an easy one to get wrong and mess up as you've just discovered! :(

Good catch that! If Github had a big thumbs up emoji imagine it here :)

@jzmaddock
Copy link
Collaborator

Diff looks good to me.

@ckormanyos
Copy link
Member Author

ckormanyos commented Feb 2, 2022

Diff looks good to me.

Thanks John (@jzmaddock)

CI on GHA is moving in the green direction. There was a hicup on drone.

I could [ci skip] a trivial commit to restart it or (when/if) GHA runs green. Or I could also simply merge to develop. But before deciding that, ... Could I get a second opinion on this.

To me, it looks like spurious runner problems on drone. What's your take?

@jzmaddock
Copy link
Collaborator

Yep spurious: couldn't connect to ubuntu.com for some reason.

@ckormanyos
Copy link
Member Author

ckormanyos commented Feb 2, 2022

Yep spurious: couldn't connect to ubuntu.com for some reason.

Thanks John. So when I merge this, Multiprecision will officially support GCC5 on -std=c++11and -std=c++14. I activated a subset of the key tests (but the most important ones) on CI. So I think it's fair to say, after this PR, we have GCC5 again.

This will get in for 1.79.

I do have interest in finding out how it looks for 4.8. But i will not press or force this issue in February. I'll take a cursory glance, however, in a future PR and we can see if it's feasible.

Cc: @jzmaddock and @mborland and @NAThompson

@ckormanyos
Copy link
Member Author

Failures in CI are:

  • Spurious failure of drone
  • Two server communication errors in GHA

Merging to develop

@ckormanyos ckormanyos merged commit 0c4ee79 into develop Feb 2, 2022
@ckormanyos ckormanyos deleted the try_to_get_gcc5_back_in_ci branch February 3, 2022 05:32
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