GEOPY-2910: b - #170
Open
domfournier wants to merge 11 commits into
Open
Conversation
This reverts commit 4f3fc71.
# Conflicts: # simpeg/dask/electromagnetics/time_domain/simulation.py
There was a problem hiding this comment.
Pull request overview
This PR targets GEOPY-2910 by adjusting how sensitivity (Jacobian) blocks are chunked/computed in Dask-based simulations, aiming to reduce chunking overhead for TEM inversions.
Changes:
- Refactors potential-fields sensitivity block computation and disk persistence to use Dask array/Zarr workflows.
- Updates TEM time-domain sensitivity assembly to reduce per-chunk overhead by stacking/solving larger derivative blocks.
- Adds an optimization flag to parallel block partitioning for TEM sensitivity computation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| simpeg/dask/potential_fields/base.py | Reworks sensitivity block evaluation, distributed-client handling, and disk (zarr) persistence logic. |
| simpeg/dask/electromagnetics/time_domain/simulation.py | Refactors derivative block handling and row assembly to reduce chunking overhead in TEM Jacobian computation. |
Suppressed comments (3)
simpeg/dask/potential_fields/base.py:83
- This
client.submit(...)path should not be used when store_sensitivities == "disk", because the disk-writing code expects dask arrays (from_delayed) rather than distributed Futures. Gate this branch so disk mode always uses the delayed/from_delayed path.
for count, block in enumerate(block_split):
if client and worker:
row = client.submit(
simpeg/dask/potential_fields/base.py:110
client.gather(rows)assumesrowsis a list of Futures, but in disk mode the loop should be building dask arrays forto_zarr. If this branch runs in disk mode it will error or do the wrong thing; gate it the same way as the submit path.
if client and worker:
kernel = client.gather(rows)
simpeg/dask/electromagnetics/time_domain/simulation.py:481
- When
local_indis empty, the functioncontinues without advancingcolm_count. That misaligns the column window (colm_count : colm_count + n_rec) for all subsequent receivers in the block, producing incorrect sensitivities.colm_countshould advance byn_recregardless of whether any data are kept for this receiver/time mask.
if len(local_ind) < 1:
row_block = np.zeros(
(len(ind_array[1]), simulation.model.size), dtype=np.float32
)
rows.append(row_block)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+74
to
78
| if client and worker: | ||
| sim = client.scatter(self, workers=worker) | ||
| else: | ||
| delayed_compute = delayed(block_compute) | ||
|
|
Comment on lines
+71
to
+73
| if client is None: | ||
| client = Client() | ||
|
|
Comment on lines
+120
to
+124
| with ProgressBar(): | ||
| j_matrix = j_matrix.to_zarr( | ||
| self.sensitivity_path, return_stored=True, compute=True | ||
| ) | ||
| return j_matrix |
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.
GEOPY-2910 - Reduce chunking of sensitivities for TEM inversions