Skip to content

279 tidy asv docs and setup#280

Open
alexallmont wants to merge 5 commits into
220-roughpy-jaxfrom
279-tidy-asv-docs-and-setup
Open

279 tidy asv docs and setup#280
alexallmont wants to merge 5 commits into
220-roughpy-jaxfrom
279-tidy-asv-docs-and-setup

Conversation

@alexallmont

Copy link
Copy Markdown
Contributor

No description provided.

- asv is now run in the root dir, not the benchmarks folder.
- In-development branch name specified here prevents asv running locally
- This needs to be set for asv run's pip to pick up roughpy_jax folder as a python package.
- These instructions may need to change when we have added a roughpy_jax pyproject.toml, but for now this helps new users get their bearings.
@alexallmont
alexallmont requested review from inakleinbottle and jackleland and removed request for inakleinbottle January 20, 2026 12:34
Comment thread asv.conf.json
"repo": ".",
"branches": [
"jl/benchmarks"
"<asv.conf set your branch name>"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not a placeholder? Surely the default branch for running benchmarks is the main branch?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a placeholder with an intentionally ugly/greppable name, so it's easy to find if you run asv without having read the README. If you do so, you will see this.

> asv run --launch-method spawn
· Unknown branch <asv.conf set your branch name> in configuration

Longer term, we should be able to get rid of branches in the json, but for now it needs to be set to your working branch name otherwise, asv complains that roughpy_jax needs to be pip-installed, i.e. it does not pick up the local roughpy_jax subdir unless this is set.

@jackleland and I hope that this will be resolved after we have got the packaging working.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, it should be set to main (or whichever branch we want to run benchmarks on long-term) but removing it defaults to main.

What confuses me is that this should have worked but built the contents of the roughpy_jax folder on that branch, rather than not building it at all, but I'm not sure we need to worry about this too much

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're writing an action for testing performance regression it is probably best to override with whatever git head initiated the action, be that a push to main branch or a PR into main or whatever. To be honest I don't really understand the execution model for ASV. It seems that one should run it on the code as it exists in the repo that is cloned. Cloning a particular branch other than this seems self-defeating.

Also, I think this is complicated by the fact that we really have two projects in the same repo that are somewhat unrelated. I'm seeing yet more evidence for separating the roughpy_jax into a separate repo.

@jackleland jackleland left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer if we shifted to running asv run from the benchmarks directory, but this is fine for now

Comment thread .gitignore
tools/vcpkg/ No newline at end of file
tools/vcpkg/

.asv/

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure on best practice here, we might want to store the results of benchmarks in the repo, but definitely not teh environment it creates for benchmarking in.

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.

3 participants