Internal benchmarking/execution time for segmentation pipeline #777
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dkuegler
force-pushed
the
feature/timeit
branch
from
6fc7256 to
955f6fc
Compare
Member
Author
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces infrastructure for measuring and logging execution time for the FastSurfer segmentation and surface pipelines, primarily via a new fs_time wrapper and a time_it helper, and wires this into run_fastsurfer.sh.
Changes:
- Extend
run_fastsurfer.shto define an execution-time log path (viaFASTSURFER_EXECTIMELOG), ensure its directory exists, and wrap major segmentation and surface commands with atime_ithelper so their runtimes are logged. - Add a new
time_itfunction and tighten error handling and batch-job management inrecon_surf/functions.sh, including more consistentPIPESTATUSchecks. - Replace the local
fs_timeimplementation with a more flexible version supporting additional flags (-a,--no-load,--debug) and improved formatting of timing output.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
run_fastsurfer.sh |
Wires in execution-time logging: documents FASTSURFER_EXECTIMELOG, sets a default exec_time_log path, ensures its directory exists, and wraps key segmentation and surface commands with time_it, while slightly reformatting command arrays and improving surface failure messaging. |
recon_surf/functions.sh |
Introduces time_it to run commands under fs_time into a timing file, adjusts RunIt/run_it/run_it_cmdf and RunBatchJobs for clearer status handling and logging, and keeps timecmd detection in sync with the new fs_time interface. |
recon_surf/fs_time |
Reworks the fs_time helper with a new option parser, configurable load-logging behavior, and improved output formatting, aligning it with the new timing pipeline while maintaining compatibility with existing usage in the recon-surf tools. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ming information is written to a new scripts/timing.log file and contains both segmentation and surface pipeline timing information. run_fastsurfer.sh - add additional flag --timeit - add additional parameter --timeit_log <filename> which defaults to scripts/timing.log - add a wrapping command that will perform the timing (wraps command in fs_time call). - the default behavior does not change - some minor formatting recon_surf/functions.sh - implements the new TimeIt function, which is the wrapper that extracts the timing information. recon_surf/fs_time - update and cleanup
…fastsurfer.sh Configuration with FASTSURFER_EXECTIMELOG environment variable possible
dkuegler
force-pushed
the
feature/timeit
branch
from
955f6fc to
fedd39d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a simple timing option of the segmentation pipeline that runs automatically.
This is tested in the docker image. The general intention would be to include execution time monitoring and improvements between libraries and python versions.
There are a couple of open questions to answer:
recon-surf.shalso switch over to logging execution times in a different file for clarity and standardization?