Skip to content

GEOPY-2910: b - #170

Open
domfournier wants to merge 11 commits into
developfrom
GEOPY-2910B
Open

GEOPY-2910: b#170
domfournier wants to merge 11 commits into
developfrom
GEOPY-2910B

Conversation

@domfournier

@domfournier domfournier commented Aug 18, 2026

Copy link
Copy Markdown

GEOPY-2910 - Reduce chunking of sensitivities for TEM inversions

Copilot AI lite review requested due to automatic review settings August 18, 2026 18:40
@github-actions github-actions Bot changed the title GEOPY-2910 b GEOPY-2910: b Aug 18, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) assumes rows is a list of Futures, but in disk mode the loop should be building dask arrays for to_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_ind is empty, the function continues without advancing colm_count. That misaligns the column window (colm_count : colm_count + n_rec) for all subsequent receivers in the block, producing incorrect sensitivities. colm_count should advance by n_rec regardless 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants