Add memory usage check to GitHub Actions workflow#75
Merged
bqminh merged 16 commits intoiqtree:masterfrom Sep 30, 2025
Merged
Conversation
56834e0 to
9fc0606
Compare
9fc0606 to
c9c09f4
Compare
bqminh
reviewed
Jul 7, 2025
Member
bqminh
left a comment
There was a problem hiding this comment.
change memory-arg and runtime-arg to column name instead of column ID, easier to read and more general.
Use 'real' time to measure wall-clock time, not user time or log file of iqtree.
Document verify_runtime.sh and verify_memory.sh, what arguments do they take as input.
update expected_memory.tsv for failing values update the verify_runtime.sh to check with usr/bin/time
f8e84d5 to
ee115db
Compare
remove expected_runtimes.tsv
1832f84 to
57e62c9
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR integrates memory usage checks into the existing CI workflows alongside runtime checks.
- Adds
verify_memory.shandverify_memory.ps1scripts to validate peak memory against thresholds. - Refactors
verify_runtime.sh/.ps1and test runners (test_iqtree.sh/.ps1) to log both real time and memory. - Updates
.github/workflows/ci.yamlto invoke memory verification for each build matrix entry.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test_scripts/verify_runtime.sh | Refactored shell runtime check to use named columns and combine logs. |
| test_scripts/verify_runtime.ps1 | Updated PowerShell runtime verification to use column names. |
| test_scripts/verify_memory.sh | New shell script to verify memory usage against thresholds. |
| test_scripts/verify_memory.ps1 | New PowerShell script for memory verification. |
| test_scripts/test_iqtree.sh | Enhanced shell test runner to log both runtime and peak memory. |
| test_scripts/test_iqtree.ps1 | Added Measure-IQTree function to capture time and memory in PowerShell. |
| test_scripts/test_data/expected_runtime.tsv | Added expected runtime threshold data. |
| test_scripts/test_data/expected_memory.tsv | Added expected memory threshold data. |
| .github/workflows/ci.yaml | Modified CI to run memory checks after runtime verification. |
Comments suppressed due to low confidence (2)
test_scripts/test_iqtree.sh:19
- [nitpick] The local variable
USERshadows the environment variable of the same name. Renaming it to something likeUSER_CPUwould improve clarity.
local REAL USER SYS PEAK_MEM MEM_MB
test_scripts/test_iqtree.ps1:78
- The backtick-quote escaping around
-m \"MIX{...}\"seems mismatched and can cause a PowerShell parse error. Consider wrapping the entire command in single quotes or using a here-string to avoid complex escaping.
Measure-IQTree "./build/iqtree3 -s test_scripts/test_data/turtle.fa -m `"MIX{GTR+FO,GTR+FO}`" --link-exchange-rates --prefix test_scripts/test_data/turtle.mix.link -seed $SEED -T 1"
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
92511ce to
5dcdf01
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.