Conversation
UXARRAY#1548 moved cartopy, matplotlib, spatialpandas and the rest of the geospatial/plotting stack out of the core dependencies and into the "geo" and "viz" extras. It did not touch benchmarks/asv.conf.json, and asv does not use the ci/environment.yml environment that runs it -- it built its own from "matrix", which listed xarray and netcdf4 and nothing else, and then pip-installed the wheel there, which since UXARRAY#1548 resolves core dependencies only. So two benchmarks have been reporting "failed" rather than a time, on every commit including main: mpas_ocean.GeoDataFrame.time_to_geodataframe OptionalDependencyNotFoundError: Failed to import: spatialpandas. mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection OptionalDependencyNotFoundError: Failed to import: cartopy, matplotlib. Point asv at ci/environment.yml instead. Benchmarks then run against the package set the project is actually tested against, and a dependency has to be added in one place rather than two -- which fixes the class of drift rather than the two instances of it that happen to be visible today. Conda now supplies the dependencies, so the project is installed with --no-deps, as the test job in ci.yml already does. Without that, pip would resolve the wheel's requirements from PyPI and, together with the --force-reinstall asv continuous needs (asv UXARRAYgh-1421), lay PyPI wheels over conda's builds on every commit. The matrix keeps only what the environment file does not carry. xarray and netcdf4 come from the file now. python-build, wheel and setuptools_scm are named explicitly because asv seeds an environment with wheel and pip only when it builds that environment without a file. Note that ci/environment.yml pins no Python version on purpose, and asv only injects "pythons" when there is no environment file, so conda now resolves the interpreter. Benchmarks move from 3.11 to whatever that resolves to, matching the environment asv itself runs in. A given comparison stays internally valid -- asv continuous installs both commits into the same environment -- but absolute timings are not comparable with results recorded before this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
asv does not benchmark in the uxarray_build environment set up by this workflow; it builds its own under benchmarks/env/, and every run gets a fresh runner, so that environment is solved and linked from scratch each time. Pointing asv at ci/environment.yml did not change that -- it changed which packages go in, not how often they go in. Restore the directory instead. Once it is present, Environment.create() returns without calling _setup() as soon as asv-env-info.json matches the configured tool, python and requirements (asv/environment.py), so the whole solve is skipped. That check does not inspect the installed packages, so the key carries everything that determines them: ci/environment.yml for the package set and asv.conf.json for the matrix and for which environment file is in use. There are deliberately no restore-keys -- a prefix match would restore an environment built from an older ci/environment.yml and asv would reuse it without noticing, which is the failure this is meant to avoid. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cmdupuis3
added
bug
ASV BenchmarkingBenchmark Comparison ResultsBenchmarks that have stayed the same:
|
cmdupuis3
changed the title
cmdupuis3
marked this pull request as ready for review
September 21, 2026 22:27
Sevans711
approved these changes
Sep 22, 2026
Sevans711
left a comment
Sevans711
left a comment
Collaborator
There was a problem hiding this comment.
Looks reasonable, and seeing that the benchmarks all ran successfully here, I'm happy to approve!
One tiny non-blocking comment: is there a reason to use actions/cache@v4 instead of actions/cache@v6 here? The asv-benchmarking.yml file uses v6.
Sevans711
added
the
benchmarking
Collaborator
Author
|
@Sevans711 Looks like an oversight, updated to v6 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1777
Overview
This PR solves #1777 and also (hopefully) optimizes the ASV CI a bit. Basically, ASV installs its own environments for benchmarking, which means it was unaware of how the optional dependencies work. Instead, we have to write its pip install line manually, but we also have the option to cache the results for the subsequent commits in the ASV runs.
PR Checklist
General
Testing & Benchmarking
Documentation and Examples
AI Disclosure
AI Usage: Claude Opus 5