-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cli Entrypoints and execution time recording #25
Conversation
If you resolve the conflict, I can merge this in. |
Resolved the conflict |
probe_src/performance_test.py
Outdated
command_to_run = "./PROBE record --make head ../flake.nix" | ||
warmup_count = 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we separate this into record --no-transcribe
and transcribe
and structure the code such that we can try 3 or 5 different commands (a list).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will do this, though I think we can't add the transcribe
command in the list since it requires the path to the raw file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added more commands in the list, reduced the warmup and benchmark iterations just to get results earlier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused by "we can't add transcribe
...". Will something like this work?
for each test_case:
time "PROBE record --no-transcribe test_case"
output times to CSV
time "PROBE transcribe test_case"
output times to CSV
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is because transcribe
will require a path to the raw data which it needs to transcribe. The --no-transcribe
command stores this raw data, and the transcribe
command later processes it. Each transcription refers to the raw data directory that was created during the recording phase.
So, the --no-transcribe
command after execution also tells us the path to the raw directory, and we give that path to the transcribe
command to process the raw data.
./PROBE record
does both execution and transcription
./PROBE record --make --no-transcribe
, only executes
./PROBE transcribe-only /path/to/raw_probe_dir --output probe_log
does only transcription
So, i meant to say that it might not be possible to hard code a transcribe command in the commands list since we don't know the path to the raw directory.
The code you shared can work but we will have to store the path to the raw directory somewhere after each --no-transcribe command.
for each test_case:
time "PROBE record --no-transcribe test_case"
output times to CSV
time "PROBE transcribe test_case"
output times to CSV
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code you shared can work but we will have to store the path to the raw directory somewhere after each --no-transcribe command.
Let's go ahead and do that, if it's simple. That will allow us to have better performance timing because we can separate the overhead of recording from transcribing.
Donee |
Please rebase onto |
3535ab2
to
d2a90c2
Compare
Hey, this pr has become very messy when i tried to rebase it, I will open a new pr for the cli points and performance_tests |
The work has been shifted to PR(#33) |
./PROBE record
does both execution and transcription./PROBE record --make --no-transcribe
, only executes./PROBE transcribe-only /path/to/raw_probe_dir --output probe_log
does only transcriptionTo view execution timings of each, we add
time
:time ./PROBE record --make head ../flake.nix
The python function (hyperfine replacement) is in the probe_src/performance_test.py
./PROBE record --no-transcribe
and./PROBE transcribe
for several tests.probe_src/performance_test.py