Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

Krzysztofpaliga/integration bench #168

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

krzysztofpaliga
Copy link

Description

Adds support for new Graphana dashboard containing new integration tests output.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

@github-actions github-actions bot added the CI label Oct 31, 2023
Copy link

@mratsim mratsim left a comment

Choose a reason for hiding this comment

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

LGTM,

Is the CPU name reported? I don't think I've seen it.

#!/bin/bash
#set -eo pipefail

k=$1
Copy link

Choose a reason for hiding this comment

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

Is it the circuit degree? might need a comment

Copy link
Author

Choose a reason for hiding this comment

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

I agree.


target_dir="$current_dir/zkevm-circuits"

printf -v _date '%(%Y-%m-%d_%H:%M:%S)T' -1
Copy link

Choose a reason for hiding this comment

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

Does this format data in UTC or local machine date?

Copy link
Author

Choose a reason for hiding this comment

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

it is set to UTC by
./weeklyBenchScripts/02_setup.sh:42:sudo timedatectl set-timezone UTC
which is called by
./integrationBenchScripts/cloud-tests-local-trigger.sh:23: ssh -i ~/.ssh/bench.pem -o StrictHostKeyChecking=no ubuntu@"$PROVER_IP" "bash -s" -- <../weeklyBenchScripts/02_setup.sh

cd "$target_dir";

mkdir ../results
logfile="$_date"--"${PROVER}"_bench-"$k".proverlog
Copy link

Choose a reason for hiding this comment

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

Not sure about using : in log filenames

Copy link
Author

Choose a reason for hiding this comment

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

I did not find any issues with it.

prepare_env
prepare_repo

kill_ssh() {
Copy link

Choose a reason for hiding this comment

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

This probably needs an explanation to why kill_ssh is needed and if it could affect a maintenance/debugging session opened that can be killed by inadvertance.

Copy link
Author

Choose a reason for hiding this comment

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

I agree.

@krzysztofpaliga
Copy link
Author

LGTM,

Is the CPU name reported? I don't think I've seen it.

No, just the number of cores. CPU name was never requested.

Comment on lines +57 to +72
instance_commitments=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*instance.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $1}')
instance_evaluations=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*instance.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $2}')
advice_commitments=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*advice.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $1}')
advice_evaluations=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*advice.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $2}')
fixed_commitments=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*fixed.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $1}')
fixed_evaluations=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*fixed.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $2}')
lookups_commitments=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*lookups.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $1}')
lookups_evaluations=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*lookups.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $2}')
equality_commitments=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*equality.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $1}')
equality_evaluations=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*equality.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $2}')
vanishing_commitments=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*vanishing.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $1}')
vanishing_evaluations=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*vanishing.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $2}')
multiopen_commitments=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*multiopen.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $1}')
multiopen_evaluations=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*multiopen.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $2}')
polycomm_commitments=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*polycomm.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $1}')
polycomm_evaluations=$(ls -t *proverlog | xargs cat | sed 's/\x1B\[[0-9;]*[JKmsu]//g' | egrep -o "|.*polycomm.*" | egrep -o [0-9]+[\ ]+[0-9]+ | awk '{print $2}')

Choose a reason for hiding this comment

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

Can this moved to a function to reduce the amount of deplication? This was automatically generated, no idea if it works:

extract_data() {
    local keyword=$1
    local column=$2
    ls -t *proverlog | xargs cat | \
    sed 's/\x1B\[[0-9;]*[JKmsu]//g' | \
    egrep -o "|.*$keyword.*" | \
    egrep -o '[0-9]+[\ ]+[0-9]+' | \
    awk -v col="$column" '{print $col}'
}

instance_commitments=$(extract_data "instance" 1)
instance_evaluations=$(extract_data "instance" 2)
advice_commitments=$(extract_data "advice" 1)
advice_evaluations=$(extract_data "advice" 2)
fixed_commitments=$(extract_data "fixed" 1)
fixed_evaluations=$(extract_data "fixed" 2)
lookups_commitments=$(extract_data "lookups" 1)
lookups_evaluations=$(extract_data "lookups" 2)
equality_commitments=$(extract_data "equality" 1)
equality_evaluations=$(extract_data "equality" 2)
vanishing_commitments=$(extract_data "vanishing" 1)
vanishing_evaluations=$(extract_data "vanishing" 2)
multiopen_commitments=$(extract_data "multiopen" 1)
multiopen_evaluations=$(extract_data "multiopen" 2)
polycomm_commitments=$(extract_data "polycomm" 1)
polycomm_evaluations=$(extract_data "polycomm" 2)

scp -i ~/.ssh/bench.pem -o StrictHostKeyChecking=no ubuntu@"$PROVER_IP":"$prover_results_dir"/*proverlog "$trigger_results_dir"/

# Enable bash Environment Variables for Prover
ssh -i ~/.ssh/bench.pem -o StrictHostKeyChecking=no ubuntu@"$PROVER_IP" "bash -s" <<EOF

Choose a reason for hiding this comment

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

Used a couple of times it seems, can be moved to a function?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants