chore(ci): refactor cpu benchmarks workflows

[soonum]

Following the same pattern as GPU benchmarks, CPU benchmarks rely on a common workflow. Weekly benchmarks are all gathered in one place. Also, all the manual launches via workflow_dispatch event are now done in one place. That way, one doesn't have to browse the workflow tree to find the right CPU benchmark to trigger.

---

This change is 

[IceTDrinker]

a few questions/remarks, thanks !

@IceTDrinker reviewed 11 of 11 files at r1, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @soonum)

---

.github/workflows/benchmark_cpu.yml line 10 at r1 (raw file):

        description: "Benchmark command to run"
        type: choice
        default: integer_multi_bit

multi bit by default for CPU ? also there is a parameter type choice lower that also allows to select multi bit stuff ?

---

.github/workflows/benchmark_cpu.yml line 55 at r1 (raw file):

        description: "Parameters type"
        type: choice
        default: multi_bit

why multi bit default for CPU ?

also isn't the parameter selection a duplicate for some given you have integer multi bit in the choice of the command to run at the start of the workflow ?

---

.github/workflows/benchmark_cpu_common.yml line 7 at r1 (raw file):

  workflow_call:
    inputs:
      backend:

is this required for the CPU ?

---

.github/workflows/benchmark_cpu_common.yml line 10 at r1 (raw file):

        type: string
        default: aws
      profile:

same

---

.github/workflows/benchmark_cpu_common.yml line 13 at r1 (raw file):

        type: string
        default: bench
      hardware_name:

same

---

.github/workflows/benchmark_cpu_common.yml line 31 at r1 (raw file):

        type: boolean
        default: false
      additional_recipe: # Make recipes to run aside the benchmarks.

for all parameters which are not used by the other workflows, why are they here ?

---

.github/workflows/benchmark_cpu_common.yml line 86 at r1 (raw file):

      - name: Set single command
        if: ${{ !contains(inputs.command, ',')}}
        run: |

these steps could all be written in python down the line if it's easier (we can select the interpreter) this will do but some transforms look like they could more easily be done in python

---

.github/workflows/benchmark_cpu_common.yml line 176 at r1 (raw file):

    concurrency:
      group: ${{ github.workflow_ref }}
      cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}

does this avoid workflow dispatches from being cancelled ?

---

.github/workflows/benchmark_cpu_common.yml line 233 at r1 (raw file):

          --bench-date "${BENCH_DATE}" \
          --walk-subdirs \
          --name-suffix avx512 \

do all benchmarks have that suffix in practice today

---

.github/workflows/benchmark_cpu_weekly.yml line 11 at r1 (raw file):

    # Weekly benchmarks will be triggered each Saturday at 1a.m.
    - cron: '0 1 * * 6'
    # Group 2

what are the groups ?

---

.github/workflows/benchmark_cpu_weekly.yml line 24 at r1 (raw file):

# TODO s'assurer que le quarterly s'exécute avec les bon paramètres (full precision, etc.)
# TODO supprimer les fichiers de workflows qui ne servent plus à rien

todo to remove ?

---

.github/workflows/benchmark_cpu_weekly.yml line 35 at r1 (raw file):

    steps:
      - name: Weekly benchmarks
        if: github.event.schedule == '0 1 * * 6' || github.event.schedule == '0 3 * * 0'

could we maybe create some variables like is_weekly_group, is quarterly etc. to make things easier to read afterwards ?

not sure if env var or something else would work in that case

[IceTDrinker]

.github/workflows/benchmark_cpu_weekly.yml line 23 at r1 (raw file):

permissions: {}

# TODO s'assurer que le quarterly s'exécute avec les bon paramètres (full precision, etc.)

to remove ?

[soonum]

Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @IceTDrinker)

---

.github/workflows/benchmark_cpu.yml line 10 at r1 (raw file):

Previously, IceTDrinker wrote…

multi bit by default for CPU ? also there is a parameter type choice lower that also allows to select multi bit stuff ?

Thinking about it we can set no default, so it forces users to actively select the bench they want to run.

Regarding the parameters type choice it's not related, at least for integer bench, this is useful for shortint and core_crypto that don't have dedicated recipe to run multi_bit benchmarks.
That being said I can remove the *integer_multi_bit choice and make sure that the Makefile recipe take the params type as discriminant.

---

.github/workflows/benchmark_cpu.yml line 55 at r1 (raw file):

Previously, IceTDrinker wrote…

why multi bit default for CPU ?

also isn't the parameter selection a duplicate for some given you have integer multi bit in the choice of the command to run at the start of the workflow ?

Leftovers from GPU workflow copy/paste. Switching it to classical

---

.github/workflows/benchmark_cpu_common.yml line 7 at r1 (raw file):

Previously, IceTDrinker wrote…

is this required for the CPU ?

Nope, you're right we can remove all three of them for now.

---

.github/workflows/benchmark_cpu_common.yml line 31 at r1 (raw file):

Previously, IceTDrinker wrote…

for all parameters which are not used by the other workflows, why are they here ?

I don't get your point on this one.

---

.github/workflows/benchmark_cpu_common.yml line 86 at r1 (raw file):

Previously, IceTDrinker wrote…

these steps could all be written in python down the line if it's easier (we can select the interpreter) this will do but some transforms look like they could more easily be done in python

You mean a little script located in ci/ folder that does all the parsing and append directly into GITHUB_ENV file ?

---

.github/workflows/benchmark_cpu_common.yml line 176 at r1 (raw file):

Previously, IceTDrinker wrote…

does this avoid workflow dispatches from being cancelled ?

Nope, I need to remove this cancellation condition since we have only workflow_call event that trigger this workflow.
So we cannot distinguish between schedule and workflow_dispatch triggers.

---

.github/workflows/benchmark_cpu_common.yml line 233 at r1 (raw file):

Previously, IceTDrinker wrote…

do all benchmarks have that suffix in practice today

GPU benchmarks don't have it.
But we only need it for FFT benchmarks I guess, since we're always using AVX512 on our bench machines.

---

.github/workflows/benchmark_cpu_weekly.yml line 11 at r1 (raw file):

Previously, IceTDrinker wrote…

what are the groups ?

What do you mean ? Maybe the comment line after schedule: declaration isn't clear enough ?

---

.github/workflows/benchmark_cpu_weekly.yml line 24 at r1 (raw file):

Previously, IceTDrinker wrote…

todo to remove ?

Yes.

---

.github/workflows/benchmark_cpu_weekly.yml line 35 at r1 (raw file):

Previously, IceTDrinker wrote…

could we maybe create some variables like is_weekly_group, is quarterly etc. to make things easier to read afterwards ?

not sure if env var or something else would work in that case

I'll try to factorize that.

[IceTDrinker]

@IceTDrinker reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @IceTDrinker and @soonum)

---

.github/workflows/benchmark_cpu.yml line 10 at r1 (raw file):

Previously, soonum (David Testé) wrote…

Thinking about it we can set no default, so it forces users to actively select the bench they want to run.

Regarding the parameters type choice it's not related, at least for integer bench, this is useful for shortint and core_crypto that don't have dedicated recipe to run multi_bit benchmarks.
That being said I can remove the *integer_multi_bit choice and make sure that the Makefile recipe take the params type as discriminant.

it's just that I don't recall how benchmarks are organized, feels a bit weird to have a dedicated target for multi bit integer benchmarks ?

---

.github/workflows/benchmark_cpu_common.yml line 31 at r1 (raw file):

Previously, soonum (David Testé) wrote…

I don't get your point on this one.

I meant for the backend and stuff, and this one I don't think I see it being provided by the calling workflows ? but maybe I just missed it

---

.github/workflows/benchmark_cpu_common.yml line 86 at r1 (raw file):

Previously, soonum (David Testé) wrote…

You mean a little script located in ci/ folder that does all the parsing and append directly into GITHUB_ENV file ?

can be, it's just that you could even write the python directly in the workflow file if it ever makes sense, having some scripts for "trivial" data preparation may be too cumbersome

---

.github/workflows/benchmark_cpu_weekly.yml line 11 at r1 (raw file):

Previously, soonum (David Testé) wrote…

What do you mean ? Maybe the comment line after schedule: declaration isn't clear enough ?

I mean what does group1 do ? what does group 2 do ? why do we have groups here ?

[soonum]

Reviewable status: 11 of 13 files reviewed, 3 unresolved discussions (waiting on @IceTDrinker)

---

.github/workflows/benchmark_cpu.yml line 10 at r1 (raw file):

Previously, IceTDrinker wrote…

it's just that I don't recall how benchmarks are organized, feels a bit weird to have a dedicated target for multi bit integer benchmarks ?

Agreed but we have dedicated recipe (even for GPU versions) so that people doesn't have to explicitly pass BENCH_PARAM_TYPE=mulit_bit as env var to run the multi-bit version of the benchmarks.
We can have only integer/signed_integer targets on this bench though, and I make sure that we passe the params_type inputs as env var of the recipe. WDYT ?

---

.github/workflows/benchmark_cpu_common.yml line 31 at r1 (raw file):

Previously, IceTDrinker wrote…

I meant for the backend and stuff, and this one I don't think I see it being provided by the calling workflows ? but maybe I just missed it

Yes it's provided in for the weekly boolean bench, in order to run make measure_boolean_key_sizes

---

.github/workflows/benchmark_cpu_common.yml line 86 at r1 (raw file):

Previously, IceTDrinker wrote…

can be, it's just that you could even write the python directly in the workflow file if it ever makes sense, having some scripts for "trivial" data preparation may be too cumbersome

Done, I've tested all the cases using the placeholder_workflow with the inputs we provide.

---

.github/workflows/benchmark_cpu_weekly.yml line 11 at r1 (raw file):

Previously, IceTDrinker wrote…

I mean what does group1 do ? what does group 2 do ? why do we have groups here ?

They are just different launch time (one is on Saturday, one is on Sunday) to avoid launching all the benchmarks at once.
# Weekly schedules are separated in two groups to avoid spawning too many the machines at once thus risking resource shortages