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

Refactor CI workflows #1798

Merged
merged 32 commits into from
Jun 12, 2024
Merged

Refactor CI workflows #1798

merged 32 commits into from
Jun 12, 2024

Conversation

joaander
Copy link
Member

@joaander joaander commented Jun 5, 2024

Description

Move away from jinja templated workflows to reusable workflows

A future PR will migrate to base Ubuntu templates with micromamba provided build dependencies. With micromamba and conda lock files, bumping a single dependency (e.g. numpy) no longer requies long docker image builds. However, moving from jinja templates to reusable workflows is enough for one PR on its own.

Motivation and context

Make it easier to update workflows and allow dependabot to update GitHub actions versions.

How has this been tested?

CI checks.

Change log

Changed:

  • Test with gcc14, clang17, and clang18. No longer test with clang10
    (#1798 <https://github.com/glotzerlab/hoomd-blue/pull/1798>__).

Checklist:

Release test configurations have not yet been added.
@joaander joaander added validate Execute long running validation tests on pull requests release Build and unit test all support compiler/python configurations labels Jun 5, 2024
@joaander joaander force-pushed the refactor-ci branch 9 times, most recently from 79e9a2b to e11b6b3 Compare June 6, 2024 17:20
Use GitHub provided runners when jetstream2 is not available.
@joaander
Copy link
Member Author

clang18 adds warnings when variable length arrays are placed on the stack. HOOMD-blue code did so in a variety of places. I replaced these with std::vector in the MPI code. Benchmark before and after (mpirun -n 24 python3 -m hoomd_benchmarks.md_pair_lj --device CPU -v -N 64000 --warmup_steps 10000 --benchmark_steps 1000 --repeat 10):

  • Before: 269 TPS
  • After: 269 TPS
    The MPI std::vector allocations are once per timestep (or less often?), so they do not impose an overhead.

I used a large fixed size stack array in the convex polyhedron support function. This code is called in the innermost loop, so std::vector will severely impact performance here. I added an error check to prevent stack overflows. The max size is 4096 vertices which should be much larger than any reasonable use-case for convex polyhedron overlap checks.
Benchmark (python3 -m hoomd_benchmarks.hpmc_octahedron --device=CPU -v -N 1000 --repeat 10)

  • Before: 866 sweeps/s
  • After: 919 sweeps/s
    With the now force inlined methods, the fixes stack allocation occurs only once per timestep. Even when including the forceinline changes, the fixed size stack allocation is faster than the variable length arrays.

@joaander joaander marked this pull request as ready for review June 10, 2024 17:13
@joaander joaander requested review from a team, tcmoore3, SchoeniPhlippsn and cbkerr and removed request for a team June 10, 2024 17:13
@joaander joaander merged commit d629a0b into trunk-minor Jun 12, 2024
75 checks passed
@joaander joaander deleted the refactor-ci branch June 12, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Build and unit test all support compiler/python configurations validate Execute long running validation tests on pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants