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

CI: enable optimisations, use master of reporting libraries #2186

Merged
merged 7 commits into from
Jan 26, 2023

Conversation

olupton
Copy link
Collaborator

@olupton olupton commented Jan 24, 2023

This PR aims to make two changes:

  • The simulation stack CI now uses development versions of libsonata-report and reportinglib. Previously, development versions of NEURON were tested against the latest releases of libsonata-report and reportinglib.
  • The GitLab-based CI now builds with more compiler optimisations enabled than before.

In pursuit of these two goals, a few other changes crept in:

  • The CI_BRANCHES syntax in PR descriptions on GitHub now supports setting BLUECONFIGS_BRANCH.
  • solve_interleaved2 gains two workarounds for issues in nvc++ 22.3:
    • an if (nt->compute_gpu) condition is made explicit, as this was being ignored. some other re-organisation (solve_interleaved2_loop_body) avoids code duplication.
    • an explicit map clause is added to nrn_pragma_omp(target teams loop ...) to avoid a crash where nvc++
      appeared to mis-deduce the length of an array
  • IntervalFireStats: compute stats instead of hashes tqperf#14 modifies the tqperf tests not to use cryptographic hashes but instead compute basic statistics of network events. this allows the test to have a fuzzy comparisons, which allows it to pass with compiler optimisations enabled

TODO:

CI_BRANCHES:SPACK_BRANCH=olupton/neuron-fast-debug,BLUECONFIGS_BRANCH=olupton/neuron-fast-debug

@olupton
Copy link
Collaborator Author

olupton commented Jan 24, 2023

In the spack_setup job of the NEURON pipeline:

==> reportinglib: resolved branch master to 59ed05db352ce470cd373dceb651785bf93f7b4a
==> nmodl: resolved branch master to ac272785dc444c8444b085d121f08b7575bb6647
==> libsonata-report: resolved branch master to b767078972c375408faee8f348b794039a994493
==> neuron@develop: remove branch/commit/tag
==> neuron@develop: use commit="18aff31877fab8e9cee8f97395fee0d482333122"
==> neuron@develop: add preferred=True
==> reportinglib@develop: remove branch/commit/tag
==> reportinglib@develop: use commit="59ed05db352ce470cd373dceb651785bf93f7b4a"
==> reportinglib@develop: add preferred=True
==> nmodl@develop: remove branch/commit/tag
==> nmodl@develop: use commit="ac272785dc444c8444b085d121f08b7575bb6647"
==> nmodl@develop: add preferred=True
==> libsonata-report@develop: remove branch/commit/tag
==> libsonata-report@develop: use commit="b767078972c375408faee8f348b794039a994493"
==> libsonata-report@develop: add preferred=True

and in the spack_setup job of the child simulation stack pipeline:

==> libsonata-report@develop: remove branch/commit/tag
==> libsonata-report@develop: use commit="b767078972c375408faee8f348b794039a994493"
==> libsonata-report@develop: add preferred=True
==> nmodl@develop: remove branch/commit/tag
==> nmodl@develop: use commit="ac272785dc444c8444b085d121f08b7575bb6647"
==> nmodl@develop: add preferred=True
==> neuron@develop: remove branch/commit/tag
==> neuron@develop: use commit="18aff31877fab8e9cee8f97395fee0d482333122"
==> neuron@develop: add preferred=True
==> reportinglib@develop: remove branch/commit/tag
==> reportinglib@develop: use commit="59ed05db352ce470cd373dceb651785bf93f7b4a"
==> reportinglib@develop: add preferred=True

so the change is being propagate correctly.

This will inevitably involve building reportinglib and libsonata-report at least once every time (once each with Intel and NVHPC if we don't force otherwise), but we should check before merging that does not inadvertently cause more of the world to be rebuilt.

@olupton olupton force-pushed the olupton/use-default-branches-of-reporting-libraries branch 3 times, most recently from e32f653 to c6c5a9b Compare January 24, 2023 11:14
@codecov-commenter
Copy link

codecov-commenter commented Jan 24, 2023

Codecov Report

Merging #2186 (3355611) into master (7d905a3) will decrease coverage by 0.01%.
The diff coverage is 96.15%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #2186      +/-   ##
==========================================
- Coverage   55.72%   55.72%   -0.01%     
==========================================
  Files         615      615              
  Lines      123849   123849              
==========================================
- Hits        69018    69016       -2     
- Misses      54831    54833       +2     
Impacted Files Coverage Δ
src/coreneuron/permute/cellorder.cpp 99.31% <96.15%> (-0.69%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@azure-pipelines
Copy link

✔️ c6c5a9b9da546bbf5e7820d9a5828990a7f224b4 -> Azure artifacts URL

@olupton olupton force-pushed the olupton/use-default-branches-of-reporting-libraries branch from 50809a8 to b172934 Compare January 24, 2023 12:40
@azure-pipelines
Copy link

✔️ b17293416a610ff452e5984771aaeb7496a50657 -> Azure artifacts URL

@azure-pipelines
Copy link

✔️ 544bbbc9f89f7b11c5ef645410f45783680c92d9 -> Azure artifacts URL

@azure-pipelines
Copy link

✔️ fce8c70797f3a0aedc6ff11d7b8b00ed1afb6732 -> Azure artifacts URL

@olupton
Copy link
Collaborator Author

olupton commented Jan 25, 2023

GPU failures potentially related to BlueBrain/CoreNeuron#885

This should be picked up from GitHub PR descriptions using the
CI_BRANCHES syntax, or if it is set manually when launching a pipeline
using the GitLab UI.
- Add explicit mapping clauses to avoid a crash in test-solver when
  compiled with optimisations enabled.
- Use an explicit if (nt->compute_gpu) block instead of relying on if
  clauses in OpenACC and OpenMP directives; nvc++ 22.3 does not seem to
  respect these clauses at least in some cases.
- To avoid code duplication, introduce a solve_interleaved2_loop_body
  function that combines the triangularisation and back substitution
  steps.
- Because the compiler cannot deal with many layers of function
  calls in device code, manually inline function calls into
  solve_interleaved2_loop_body.
@olupton olupton force-pushed the olupton/use-default-branches-of-reporting-libraries branch from fce8c70 to b6620ae Compare January 26, 2023 12:05
@azure-pipelines
Copy link

✔️ b6620ae -> Azure artifacts URL

@olupton olupton changed the title libsonata-report, reportinglib: default to building master CI: enable optimisations, use master of reporting libraries Jan 26, 2023
@olupton olupton marked this pull request as ready for review January 26, 2023 15:21
@azure-pipelines
Copy link

✔️ 6c40a0d -> Azure artifacts URL

@azure-pipelines
Copy link

✔️ 3355611 -> Azure artifacts URL

@olupton olupton merged commit 9531fc5 into master Jan 26, 2023
@olupton olupton deleted the olupton/use-default-branches-of-reporting-libraries branch January 26, 2023 17:54
alexsavulescu pushed a commit that referenced this pull request Jan 27, 2023
* libsonata-report, reportinglib: default to building master
* libsonata-report, reportinglib: single %gcc build
* neuron: no more debug variant, prefer build_type=FastDebug
* BLUECONFIGS_BRANCH: support dynamic setting
  This should be picked up from GitHub PR descriptions using the
  CI_BRANCHES syntax, or if it is set manually when launching a
  pipeline using the GitLab UI.
* [email protected] + OpenMP: fixes including for -O1
- Add explicit mapping clauses to avoid a crash in test-solver when
  compiled with optimisations enabled.
- Use an explicit if (nt->compute_gpu) block instead of relying on if
  clauses in OpenACC and OpenMP directives; nvc++ 22.3 does not seem to
  respect these clauses at least in some cases.
- To avoid code duplication, introduce a solve_interleaved2_loop_body
  function that combines the triangularisation and back substitution
  steps.
- Because the compiler cannot deal with many layers of function
  calls in device code, manually inline function calls into
  solve_interleaved2_loop_body.
* tqperf: bump past tqperf#14.
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.

4 participants