Fix memory copies getz#120
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #120 +/- ##
==========================================
+ Coverage 98.46% 98.88% +0.42%
==========================================
Files 22 22
Lines 975 989 +14
Branches 98 103 +5
==========================================
+ Hits 960 978 +18
+ Misses 8 6 -2
+ Partials 7 5 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR addresses excess memory usage when simulating with per-antenna beam selection by avoiding fancy-indexing copies in Z-matrix formation, and extends the chunk-sizing logic with a configurable free-memory safety margin.
Changes:
- Avoid fancy-indexing beam selection in
ZMatrixCalcto prevent large temporary allocations whenbeam_idxis used. - Add a
memory_bufferparameter to CPU/GPUsimulate()and propagate it into chunk estimation utilities. - Adjust chunk-estimation thresholds to account for
memory_buffer, and tweak docstring composition in the GPU entrypoint.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
src/matvis/gpu/gpu.py |
Adds memory_buffer to GPU simulate and passes it into chunk sizing. |
src/matvis/cpu/cpu.py |
Adds memory_buffer to CPU simulate and documents the new parameter. |
src/matvis/core/getz.py |
Replaces beam fancy-indexing with a per-antenna loop to avoid memory copies. |
src/matvis/_utils.py |
Extends chunk-sizing helpers with memory_buffer and uses it in the stopping criterion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot please re-review given the rebase |
…and GPU simulate Agent-Logs-Url: https://github.com/HERA-Team/matvis/sessions/b7466356-8902-46eb-b67c-a9b2b60b6fdf Co-authored-by: steven-murray <1272030+steven-murray@users.noreply.github.com>
Re-reviewed after the rebase. The remaining open items were:
Both are now addressed in commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
for more information, see https://pre-commit.ci
tyler-a-cox
left a comment
There was a problem hiding this comment.
This is all looking good to me. Thanks @steven-murray!
Stacks on #119
Fixes an issue where memory copies were made when using multiple beams.