Skip to content

ci/eval: add a shebang to the stats comparison script#437697

Closed
philiptaron wants to merge 1 commit intoNixOS:masterfrom
philiptaron:add-shebang-to-ci-perf-script
Closed

ci/eval: add a shebang to the stats comparison script#437697
philiptaron wants to merge 1 commit intoNixOS:masterfrom
philiptaron:add-shebang-to-ci-perf-script

Conversation

@philiptaron
Copy link
Contributor

This isn't for CI, just for humans. I didn't do anything fancy like ensure that I was using the correct CI pinned json, but it shouldn't matter: this is just to make it easier to run the script.

Things done

This isn't for CI, just for humans.
@philiptaron philiptaron requested a review from a team August 27, 2025 22:43
@@ -1,3 +1,6 @@
#! /usr/bin/env nix-shell
#! nix-shell -i "python3 -I" -p "python3.withPackages(ps: with ps; [ numpy pandas scipy ])"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the production caller.

(python3.withPackages (
ps: with ps; [
numpy
pandas
scipy
]
))

Copy link
Member

Choose a reason for hiding this comment

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

you could refer to current nixpkgs checkout here instead of relying on NIX_PATH, i.e. -I nixpkgs ../.. (don't remember the exact syntax in this case).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's the expression that Silvan suggested in #425551:

#!/usr/bin/env nix-shell
#!nix-shell -i "python3 -I" --expr 'with import ../.. {}; with pkgs; mkShellNoCC { packages = [ python3.withPackages(ps: with ps; [ numpy pandas scipy ]) ] }'

Likely can be golfed substantially.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions backport release-25.05 labels Aug 27, 2025
@wolfgangwalther
Copy link
Contributor

Is this something of hypothetical or actual value? Aka, why would you try to run this script isolated? You will need to set up the before / after folders correctly to make any sense of it - but once you do, it would be very easy to just run nix-build ci -A eval.compare ... with the right arguments. This has a lot better interface, because you need to pass these folders as arguments, while the script expects them as environment variables.

@Mic92
Copy link
Member

Mic92 commented Aug 28, 2025

Is this something of hypothetical or actual value? Aka, why would you try to run this script isolated? You will need to set up the before / after folders correctly to make any sense of it - but once you do, it would be very easy to just run nix-build ci -A eval.compare ... with the right arguments. This has a lot better interface, because you need to pass these folders as arguments, while the script expects them as environment variables.

Maybe the pr was made because this option wasn't obvious?

https://github.com/NixOS/nixpkgs/tree/master/ci/eval we only document the full eval here.

@philiptaron
Copy link
Contributor Author

philiptaron commented Aug 28, 2025

Is this something of hypothetical or actual value? Aka, why would you try to run this script isolated? You will need to set up the before / after folders correctly to make any sense of it - but once you do, it would be very easy to just run nix-build ci -A eval.compare ... with the right arguments. This has a lot better interface, because you need to pass these folders as arguments, while the script expects them as environment variables.

Yes, I was trying to run this script directly in order to gather locally changes to #431756. I am running it directly because I found the combinedDir parameter irritating to satisfy.

Here's the workflow. I'd love to hear the better version and what I'm doing that's ... dumb. 🧠

  1. On the baseline version, run nix-build ci/ -A eval.full --outlink baseline. This directory contains both before and after subdirectories, but they're the same.
  2. I make some changes, doing a quick test to see that I haven't changed the outpath (supposed to be pure refactor)
  3. On the new version, run nix-build ci/ -A eval.full --outlink experiment-1-no-optional (or whatever the experiment is)
  4. Compare with BEFORE_DIR=baseline/before AFTER_DIR=experiment-1-no-optional/after ci/eval/compare/cmp-stats.py > experiment-1-no-optional/results.md
  5. Observe that file in the terminal with frogmouth experiment-1-no-optional/results.md.

I do that on a loop, making new experiment directories in the worktree but leaving the baseline.

@wolfgangwalther
Copy link
Contributor

I ran running it directly because I found the combinedDir parameter irritating to satisfy.

OK, I see the problem. You essentially have two "no-op" combined directories, because each full creates one. The current full thing is not really useful, I think.

I think we can make that better... I'll work something out.

@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Aug 28, 2025

Here's the workflow. I'd love to hear the better version and what I'm doing that's ... dumb. 🧠

1. On the baseline version, run `nix-build ci/ -A eval.full --outlink baseline`. This directory contains both `before` and `after` subdirectories, but they're the same.

2. I make some changes, doing a quick test to see that I haven't changed the outpath (supposed to be pure refactor)

3. On the new version, run `nix-build ci/ -A eval.full --outlink experiment-1-no-optional` (or whatever the experiment is)

4. Compare with `BEFORE_DIR=baseline/before AFTER_DIR=experiment-1-no-optional/after ci/eval/compare/cmp-stats.py > experiment-1-no-optional/results.md`

5. Observe that file in the terminal with `frogmouth experiment-1-no-optional/results.md`.

I do that on a loop, making new experiment directories in the worktree but leaving the baseline.

With #437934, you should be able to:

On the baseline: nix-build ci -A eval.all --out-link baseline

On the new version: nix-build ci -A eval.full --arg baseline $(readlink ./baseline)

And then take the respective file from the result of that.

@philiptaron philiptaron deleted the add-shebang-to-ci-perf-script branch August 28, 2025 16:22
@philiptaron
Copy link
Contributor Author

Closing after #437934 was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants