gh-1088: Add realistic benchmark tests - #1098
Conversation
c2cd7e6 to
19ec5c0
Compare
cd255a4 to
90d3f20
Compare
1c6f375 to
a77d3e0
Compare
568254e to
bcb579a
Compare
a77d3e0 to
6b294a1
Compare
6b294a1 to
696463b
Compare
f474cc5 to
9329122
Compare
|
@paddyroddy I'm not sure why readthedocs is failing. Any ideas? |
I've been trying to fix it, no luck yet |
paddyroddy
left a comment
There was a problem hiding this comment.
Have gone through. Fixed typos. And tried to fix readthedocs.
| array_api_strict.set_array_api_strict_flags(api_version="2025.12") | ||
| jax.config.update("jax_enable_x64", val=True) |
There was a problem hiding this comment.
This whole file seems very similar to tests/fixtures/array_backends.py. Is there a way we can reduce the duplication?
There was a problem hiding this comment.
I think the only similarity is the if blocks, i.e.:
if not ARRAY_BACKEND or ARRAY_BACKEND == "numpy":
xp_available_backends["numpy"] = np
...Everything else is new, no?
There was a problem hiding this comment.
I guess all of this?
glass/tests/fixtures/array_backends.py
Lines 22 to 96 in faec065
There was a problem hiding this comment.
I decided not to do that here and just always import numpy, jax, array_api_strict, etc. In fact I can probably remove array_api_strict as I doubt we care about benchmarking that.
| *args: tuple[Any, ...], | ||
| xp: ModuleType, | ||
| **kwargs: dict[str, Any], |
There was a problem hiding this comment.
This seems very unusual. Surely we go function_to_benchmark, xp, args, kwargs?
There was a problem hiding this comment.
I know what you mean. The reason for this is I want to ensure xp is always provided as a named variable. If I have xp, *args, **kwargs then the first positional arg will be assumed to be xp but it doesn't have to be.
There was a problem hiding this comment.
Does xp have to be named?
How about:
def run_benchmark(
function_to_benchmark: FunctionType,
xp: ModuleType,
*,
args: tuple[Any, ...] = (),
kwargs: dict[str, Any] | None = None,
):There was a problem hiding this comment.
Doesn't have to be. However, if I then call run_benchmark like this:
x: FloatArray
# some logic to set `x` to and define `some_benchmark`
run_benchmark(
some_benchmark,
x,
)Then xp in run_benchmark will be x.
There was a problem hiding this comment.
Clearly. Is that not a fault with run_benchmark? Not entirely sure what you're trying to do here.
There was a problem hiding this comment.
What about
def run_benchmark(
function_to_benchmark: Callable[..., Any],
*,
xp: ModuleType,
args: tuple[Any, ...] = (),
kwargs: dict[str, Any] | None = None,
) -> None:
I've pushed another attempted fix. Will request a Copilot review also. |
There was a problem hiding this comment.
Pull request overview
This PR adds a new “realistic” benchmarking workflow (scripts + docs) to measure GLASS performance across local machines and HPC clusters (e.g., Archer2/Cosma8/AAC6), and makes a few array-backend compatibility tweaks in core lensing/shell code to support those benchmarks.
Changes:
- Introduces a new
benchmarks/suite with a lensing benchmark, shared utilities, and cluster submission scripts/docs. - Updates dependency groups for benchmark + GPU environments and adjusts example execution to install
glass-ext-camb. - Improves array-backend compatibility in lensing/shear code and shells tests.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/regression/archer2/README.md | Repoints setup instructions to shared benchmark docs (link needs correction). |
| tests/core/test_shells.py | Adjusts assertion construction for backend compatibility. |
| pyproject.toml | Adds benchmark/GPU dependency groups and lint ignores; modifies examples extra. |
| noxfile.py | Ensures glass-ext-camb is installed for running example notebooks. |
| glass/shells.py | Forces RadialWindow.zeff to be a Python float. |
| glass/lensing.py | Moves shear computation utilities toward backend-agnostic array ops. |
| benchmarks/README.md | Adds benchmark runner documentation (contains a small typo). |
| benchmarks/lensing.py | Adds a realistic lensing benchmark script. |
| benchmarks/cosma8/submit-gpu.sh | Adds Cosma8 GPU Slurm submission script (path handling needs tightening). |
| benchmarks/cosma8/submit-cpu.sh | Adds Cosma8 CPU Slurm submission script (path handling needs tightening). |
| benchmarks/cosma8/README.md | Adds Cosma8 benchmark instructions (contains a broken relative link). |
| benchmarks/benchmark_utils.py | Adds backend selection + timing utilities and a Cosmology wrapper (contains a bug). |
| benchmarks/archer2/submit-gpu.sh | Adds Archer2 GPU Slurm submission script (path handling needs tightening). |
| benchmarks/archer2/submit-cpu.sh | Adds Archer2 CPU submission script (path handling needs tightening). |
| benchmarks/archer2/setup-gpu-env.sh | Adds Archer2 GPU module/environment setup script (minor typo). |
| benchmarks/archer2/README.md | Adds Archer2 benchmark instructions (contains a broken relative link). |
| benchmarks/aac6/submit-gpu.sh | Adds AAC6 GPU Slurm submission script (path handling needs tightening). |
| benchmarks/aac6/README.md | Adds AAC6 benchmark instructions (contains a broken relative link). |
| benchmarks/init.py | Adds benchmarks package marker. |
| .typos.toml | Adds a shell-word exception for “HSA”. |
| .gitignore | Ignores healpy-data directory used for offline runs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ### GPU prerequisites | ||
|
|
||
| For the gpu benchmark, there is an additional dependency group `aac6-gpu` which |
There was a problem hiding this comment.
| For the gpu benchmark, there is an additional dependency group `aac6-gpu` which | |
| For the GPU benchmark, there is an additional dependency group `aac6-gpu` which |
|
|
||
| ### GPU prerequisites | ||
|
|
||
| For the gpu benchmark, there is an additional dependency group `archer2-gpu` |
There was a problem hiding this comment.
| For the gpu benchmark, there is an additional dependency group `archer2-gpu` | |
| For the GPU benchmark, there is an additional dependency group `archer2-gpu` |
| the exact "ideal" benchmark. | ||
|
|
||
| [benchmarks/archer2/submit-gpu.sh](./submit-gpu.sh) specifically will submit a | ||
| job to the archer2 amd gpu testbed queue. |
There was a problem hiding this comment.
| job to the archer2 amd gpu testbed queue. | |
| job to the ARCHER2 AMD GPU testbed queue. |
| # Setup environment | ||
| source "$GLASS_DIR/benchmarks/archer2/setup-gpu-env.sh" | ||
|
|
||
| # Flags to maximise jax gpu performance |
There was a problem hiding this comment.
| # Flags to maximise jax gpu performance | |
| # Flags to maximise JAX GPU performance |
|
|
||
| ### GPU prerequisites | ||
|
|
||
| For the gpu benchmark, there is an additional dependency group `cosma8-gpu` |
There was a problem hiding this comment.
| For the gpu benchmark, there is an additional dependency group `cosma8-gpu` | |
| For the GPU benchmark, there is an additional dependency group `cosma8-gpu` |
| module load craype-accel-amd-gfx90a | ||
| module load craype-x86-milan | ||
|
|
||
| # Ensure the rocm library and build is known to jax |
There was a problem hiding this comment.
| # Ensure the rocm library and build is known to jax | |
| # Ensure the ROCm library and build is known to JAX |
| ## Running the benchmarks | ||
|
|
||
| Benchmarks can be submitted as a batch job to slurm via the provided script. For | ||
| example to benchmark using jax with amd/rocm, run the following from the root of |
There was a problem hiding this comment.
| example to benchmark using jax with amd/rocm, run the following from the root of | |
| example to benchmark using JAX with AMD/ROCm, run the following from the root of |
| will need to install you python virtual environment. Finally, you will need to | ||
| load specific modules for the GPU benchmark. | ||
|
|
||
| Note that for the GPU benchmarks ARCHER2 only support rocm up to v0.6.x. |
There was a problem hiding this comment.
| Note that for the GPU benchmarks ARCHER2 only support rocm up to v0.6.x. | |
| Note that for the GPU benchmarks ARCHER2 only support ROCm up to v0.6.x. |
| from benchmark_utils import CosmologyWrapper, run_benchmark, xp_available_backends | ||
|
|
||
| # use the CAMB cosmology that generated the matter power spectra | ||
| import camb # ty: ignore[unresolved-import] |
There was a problem hiding this comment.
Any idea why we need the ty: ignore[unresolved-import] statements in this file?
There was a problem hiding this comment.
I've realised that benchmarks aren't currently being checked by ty. I'm going to push adding this config and fixing the few warnings.
There was a problem hiding this comment.
Are we installing these dependencies?
| "camb", | ||
| "cosmology-api", | ||
| "cosmology-compat-camb", | ||
| "glass-ext-camb", |
There was a problem hiding this comment.
I wonder if the issues we were having with glass-ext-camb was because previously they were only included as optional-dependencies, whereas now we have them as dependency-group.
|
Making a note that there seems to be an issue with OOM error on Archer2 exclusive nodes with JAX. This page contains potential solutions |
Description
Closes: #1088 #1084
Changelog entry
Added: Realistic benchmark test for benchmarking glass on various machines
Checks