Skip to content

[E2S 1.0] Established shared Pipeline / coupled model execution primitives - #1212

Open
pzharrington wants to merge 12 commits into
1.0.0-rcfrom
pzharrington/pipeline-execution
Open

pzharrington wants to merge 12 commits into
1.0.0-rcfrom
pzharrington/pipeline-execution

Conversation

@pzharrington

@pzharrington pzharrington commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Earth2Studio Pull Request

Description

Establishes the interfaces that are likely to be shared between coupled models and Pipeline execution so development of each can proceed in parallel. The end goal is to have Pipeline be the package's offering for general execution (handling distribution of work, I/O, and resume), and able to drive either more traditional single-model forecasts like in run.deterministic, or a coupled model run. See the EXECUTION_CONTRACT_SPEC.md for full details. Current code is more draft/sketch, focus should be on the interface definitions.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • The CHANGELOG.md is up to date with these changes.
  • An issue is linked to this pull request.
  • Assess and address Greptile feedback (AI code review bot for guidance; use discretion, addressing all feedback is not required).

Dependencies

@copy-pr-bot

copy-pr-bot Bot commented Oct 1, 2026

Copy link
Copy Markdown

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@pzharrington pzharrington changed the title Pzharrington/pipeline execution [E2S 1.0] Established shared Pipeline / coupled model execution primitives Oct 1, 2026
@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Disclaimer: This is AI-generated, please review response for accuracy

RetriggerConfidence Score: 1/5

[Medium risk] Adds execution framework for single-model and coupled workflows.

The PR is not ready to merge because supported forecast configurations can fail and snapshots can resume an incompatible model trajectory.

Findings

  1. P1 CUDA input device mismatch ▶
  2. P1 Source grid is not mapped ▶
  3. P1 Stochastic restart loses RNG state ▶
  4. P1 Different weights share snapshot identity ▶
  5. P2 Example uses unsupported session argument ▶
  6. P2 Nowcasting include points to removed file ▶

Summary

This PR introduces shared execution-plan, session, component, schedule, and coupling contracts, plus a direct single-model executor and tests.

  • The single-model path needs input preparation and restart-safety fixes before it can reliably replace the existing workflow.
  • The new development example uses an unsupported customization API, and one example retains a reference to the moved module.

Reviews (1) · Last reviewed commit: "Add goal line"

Comment on lines +113 to +118
return fetch_data(
self.plan.source,
time=np.array([self.item.time]),
variable=np.array(requirement.variables),
lead_time=np.array(requirement.lead_offsets),
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 CUDA input device mismatch

If an FCN model is placed on CUDA, this call still fetches its initial condition on CPU. FCN uses that input with CUDA-resident normalization buffers without moving it first, so the first forecast step fails with a device-mismatch error. The existing deterministic workflow fetches data on the model's inference device.

Comment on lines +113 to +118
return fetch_data(
self.plan.source,
time=np.array([self.item.time]),
variable=np.array(requirement.variables),
lead_time=np.array(requirement.lead_offsets),
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Source grid is not mapped

When a source's numeric grid differs from the model's declared input grid, the fetched array goes straight to the iterator. FCN rejects the coordinate mismatch, so the forecast cannot start. The existing deterministic workflow maps the source field to the model's input grid before creating the iterator.

Comment on lines +203 to +211
single_lead = input_signature.sizes["lead_time"] == 1
same_variables = _labels(input_signature, "variable") == _labels(
output_signature, "variable"
)
capability = (
CheckpointCapability.SNAPSHOTTABLE
if single_lead and same_variables
else CheckpointCapability.UNSUPPORTED
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Stochastic restart loses RNG state

StormCast has one input lead and matching input and output variables, so this check marks it as restartable. But its steps sample random latents, and the snapshot stores no RNG state. Resuming from that snapshot therefore follows a different random trajectory instead of continuing the interrupted forecast.

Comment on lines +224 to +241
model_type = f"{type(model).__module__}.{type(model).__qualname__}"
declaration = "|".join(
[
model_type,
repr(tuple(transform.identity for transform in transforms)),
spec_fingerprint(spec),
";".join(
f"{port.name}:{','.join(port.variables)}"
for port in self._output_ports.values()
),
]
)
self._identity = hashlib.sha256(declaration.encode()).hexdigest()[:16]
self.compatibility = SnapshotCompatibility(
schema_version=SNAPSHOT_SCHEMA_VERSION,
earth2studio_version=earth2studio.__version__,
plan_identity=self._identity,
component_versions={name: model_type},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Different weights share snapshot identity

Two instances of the same model class can have different weights yet receive the same identity here, because the hash includes the class and declarations but not the weights. A snapshot from one instance then passes the other's compatibility check, causing old model state to be advanced with different weights and producing an incorrect continuation.

return {"forecast": x.where(x["lat"] > 0, 0.0)}


masked = SingleModelPlan(model, source, session=NorthernHemisphere)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Example uses unsupported session argument

SingleModelPlan has no session parameter, so the runnable example stops here with a TypeError before either customization tier runs. The plan uses OutputTransform for this customization; its open method does not construct the session subclasses shown here.

# See the License for the specific language governing permissions and
# limitations under the License.

"""Forecast workflow helpers and simulation supervision.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Nowcasting include points to removed file

Moving run.py into this package leaves the StormCast ensemble example's literalinclude pointing at the removed earth2studio/run.py. Unlike the other updated examples, it can no longer include the workflow source in generated documentation.

@copy-pr-bot

copy-pr-bot Bot commented Oct 1, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@pzharrington pzharrington added the 1.0.0 Earth2Studio 1.0.0 PR label Oct 1, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.0.0 Earth2Studio 1.0.0 PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant