Skip to content

[codex] Fix GX eik input handling and subprocess errors#2143

Draft
rmchurch wants to merge 1 commit intoPlasmaControl:yge/gxfrom
rmchurch:gx
Draft

[codex] Fix GX eik input handling and subprocess errors#2143
rmchurch wants to merge 1 commit intoPlasmaControl:yge/gxfrom
rmchurch:gx

Conversation

@rmchurch
Copy link
Copy Markdown

What changed

  • force generated GX inputs to use geo_option = "eik" so DESC's plain-text geometry files match the GX input mode
  • wrap GX subprocess failures and timeouts in readable runtime errors that include stdout.gx / stderr.gx context
  • add targeted tests for input rewriting and subprocess error reporting

Why

The GX wrapper was generating a plain-text geometry file while allowing templates to keep geo_option = "nc". That made GX fail with an opaque nonzero exit, and the callback path surfaced it as a raw subprocess failure instead of a useful GX-specific error.

Impact

This makes the GX integration robust against NC-based templates and preserves actionable failure context when GX exits nonzero.

Validation

  • /global/cfs/projectdirs/m4505/rchurchi/conda-envs/desc-gx/bin/python -m pytest tests/test_external_gx.py -q

@YigitElma YigitElma self-requested a review March 29, 2026 19:21
Copy link
Copy Markdown
Collaborator

@YigitElma YigitElma left a comment

Choose a reason for hiding this comment

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

Thanks for this @rmchurch! I was naively expecting the user to put eik, but I guess people use the same template file for multiple cases.

If this is ready, I can approve/merge it to yge/gx, and create a GX wrapper PR later. Does this sound good to you?

Comment thread tests/test_external_gx.py
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We actually don't require writing tests for the desc.external module. But these look good to me. We can decide to keep them or not in the actual PR for the yge/gx branch.

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