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

WIP: Expand CI Configs #142

Merged

Conversation

insertinterestingnamehere
Copy link
Collaborator

Depends on #141

@insertinterestingnamehere
Copy link
Collaborator Author

As far as I can tell, m1 builds in github actions are for paid subscribers only. We can revisit CI for that setup later.

This is to prevent getting a bunch of failures about missing compilers
whenever ubuntu-latest gets updated to a newer version.
Instead, when a new LTS version gets released we should update the CI
configuration and the compilers being tested accordingly.
@insertinterestingnamehere
Copy link
Collaborator Author

I've been testing this on my own copy of the repo since I don't see the builds running on the main one. It should be ready now. Here's the most recent run: https://github.com/insertinterestingnamehere/qthreads/actions/runs/6579547806. There are a bunch of failures showing up there, but they're the errors documented in #144 (for clang on Linux) and #146 (for OSX). I propose we merge this and then continue work on those bugs separately.

This is currently set up to test on gcc versions 9 through 13 and clang versions 11 through 17. Testing that many compiler/config combinations is probably not necessary since qthreads seems to be less sensitive to the exact compiler version. That said, even with so many different configurations, the tests still run fairly quickly. I'm very much open to feedback here. I'm also willing to test even older compilers too if people are interested. These are just the ones that are particularly easy to install on the latest Ubuntu LTS release. It's still fairly easy to go all the way back to gcc 7 and clang 6 if we want, and I'm sure I can find a way to test even older stuff if needed.

@insertinterestingnamehere
Copy link
Collaborator Author

Okay, this should be good to go now. I just squashed a bunch of the debug commits together. An example test run will is at https://github.com/insertinterestingnamehere/qthreads/actions/runs/6656923578. There are lots of failures, but they should all be documented in issues now.

While testing I realized that there are some tests that use C++ so I expanded the test matrix to control some details of the C++ compiler configuration too. For example, now if you use clang a specific reasonable version of g++ will be installed and used to get access to an appropriate version of libstdc++. There are also some builds that tie in the latest libc++ instead of libstdc++. That's unlikely to matter right now with how limited our use of C++ is, but I like to be thorough to catch things early.

I've also added tests with varying assertion and optimization levels with the latest versions of clang and gcc. This results in a lot of different configs, but as far as I can tell, each of these settings matters WRT the errors I'm tracking right now, so I'd prefer to keep them all enabled.

I'd like to turn on -Werror for the set of builds that run with the latest clang and gcc as well, but I'll do that later. Doing so now would unnecessarily silence a lot of the errors I'm working on right now.

@insertinterestingnamehere
Copy link
Collaborator Author

Just realized I have just a few more changes to make. Converting back to a draft again.

@insertinterestingnamehere insertinterestingnamehere marked this pull request as draft October 26, 2023 19:24
…en libstdc++ and libc++ is minimal right now.
…no difference with known bugs and the use of that type of assert is extremely limited in the codebase.
@insertinterestingnamehere
Copy link
Collaborator Author

Okay, I've pruned the configs down quite a bit, but also expanded the configs that are tested on OSX. This makes the configurations thorough, but not as massive.

Key things I've removed:

  • testing at different optimization levels
  • testing with the paranoia inserts on
  • testing with libc++

All of these had no discernible affect on the known bugs at the moment. The paranoia asserts have fairly minimal interaction with the codebase. The same goes for the and libc++ vs libstdc++ distinction.

Latest sample round of tests is up at: https://github.com/insertinterestingnamehere/qthreads/actions/runs/6660843521.

@insertinterestingnamehere insertinterestingnamehere marked this pull request as ready for review October 26, 2023 23:24
@insertinterestingnamehere
Copy link
Collaborator Author

Can anyone else comment on the extent to which the distinction between the hwloc and binders topology settings matters? It seems like most of the time bugs that show up on one also show up on the other.

@insertinterestingnamehere
Copy link
Collaborator Author

@olivier-snl Can you review this one too when you get time? I don't have access to the buttons that let me request a specific reviewer yet.

@olivier-snl
Copy link
Collaborator

LGTM, but let's add the distrib scheduler to the testing while we're at it.

@insertinterestingnamehere
Copy link
Collaborator Author

Sounds good. I just added the distrib scheduler too. The latest round of tests is (currently in progress) at https://github.com/insertinterestingnamehere/qthreads/actions/runs/6709763213.

@insertinterestingnamehere
Copy link
Collaborator Author

Also added a more aggressive timeout since it appears that #162 shows up uevery once in a while on Linux/x86 as well.

@insertinterestingnamehere insertinterestingnamehere merged commit f040053 into sandialabs:main Oct 31, 2023
54 of 198 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