Adding GitHub Actions CI to execute notebooks#5
Conversation
So this adds a workflow that creates a conda-forge environment with xeus-cpp
and executes the OpenMP tutorial notebooks on every push and pull
request targeting master. Executed notebooks are uploaded as an
artifact for review.
Notes:
- An environment.yml file is added at the repo root so contributors
can reproduce the CI environment locally with:
micromamba create -f environment.yml
- The kernel requires libomp.so to be preloaded at runtime via
LD_PRELOAD; without it, the JIT fails to resolve OpenMP symbols
(omp_get_thread_num, __kmpc_fork_call, etc.).
- openmp-demo.ipynb is skipped: example5() allocates ~8 GB which
exceeds the ~7 GB RAM available on standard GitHub-hosted runners.
|
@vgvassilev let me know if you'd like any changes. |
|
Hi @mcbarton, can you take a look? |
mcbarton
left a comment
There was a problem hiding this comment.
This ai generated ci is not useful. Although it will execute the notebook, it will not actually check the outputted result is what we would expect (just because a notebook executes doesn't mean the resultant notebook is correct).
| with: | ||
| name: executed-notebooks | ||
| path: executed/ | ||
| if-no-files-found: ignore No newline at end of file |
There was a problem hiding this comment.
Surely if no files are found something went wrong with executing the notebooks. I do not understand why you would want to ignore them
There was a problem hiding this comment.
@mcbarton Okay yeah i guess we can remove the upload step entirely. With pytest --nbval doing output validation, executed-notebook artifacts aren't needed and the test result is the validation. So both the version pin and the if-no-files-found settings are moot ??
|
|
||
| - name: Execute notebooks | ||
| run: | | ||
| set +e # don't stop on first failure; we want to see all results |
There was a problem hiding this comment.
If any notebook fails to execute, we would want the ci to fail. Why run all the notebooks if the ci is going to fail anyway?
There was a problem hiding this comment.
Jeez, this was a typo, i thing i wrote the wrong piece of line here,lol.
| fi | ||
|
|
||
| echo "=== Executing $nb ===" | ||
| LD_PRELOAD="$CONDA_PREFIX/lib/libomp.so" \ |
There was a problem hiding this comment.
You don't need this LD_PRELOAD. You need the clang resource directory in your environment. Its a known bug in CppInterOp.
|
@mcbarton Was not fully AI generated, i get your frustration of reviewing shitty AI generated code, some were my mistakes of writing some shitty script and minor mistakes which were not removed during testing, sorry for that, and yeah , thanks for the feedback, so yeah Would u mind looking at this and maybe some feedbacks on this, i don't want to ruin the thing by creating a vauge commit again ? |
So this adds a workflow that creates a conda-forge environment with xeus-cpp and executes the OpenMP tutorial notebooks on every push and pull request targeting master. Executed notebooks are uploaded as an artifact for review.
Notes: