Skip to content

Revive DIII-D get_n1_bradial_parameters#500

Open
yumouwei wants to merge 8 commits intodevfrom
wei/d3d-onsbradial
Open

Revive DIII-D get_n1_bradial_parameters#500
yumouwei wants to merge 8 commits intodevfrom
wei/d3d-onsbradial

Conversation

@yumouwei
Copy link
Copy Markdown
Contributor

@yumouwei yumouwei commented Dec 18, 2025

Implemented changes

  • Reimplement D3DPhysicsMethods.get_n1_bradial_parameters. This method was previously moved to drafts/machine/d3d/physics.py in v0.8 (Add docstrings for modules/classes/methods #325) and later removed alongside all other draft scripts in v0.11 (Remove draft scripts #420).
  • Create D3DPhysicsMethods.get_btor and use it in get_n1_bradial_parameters and get_n1rms.
  • These new physics methods return n_equal_1_mode, n_equal_1_normalized (tested & XFAIL), and btor (not tested)

Notes on testing

  • n_equal_1_mode and n_equal_1_normalized are both marked as XFAIL as all 4 testing shots fall within the range of shots where the raw signal data need to be retrieved from files previously stored on iris.
  • I have checked these two signals with the last shot (177061) in the DISRUPTION_WARNING table and the outputs from this physics method match the data in the table.
  • The outputs for shot 175552 (referenced in Nucl. Fusion 59 (2019) 096016) do not match the stored data in the DISRUPTION_WARNING table. I found out that these stored data match those from the legacy DUD system (fallback data source in the method's logic) instead of the ONFR system which is available for this shot.

Future TODO

@gtrevisan gtrevisan changed the title Revive D3D get_n1_bradial_parameters Revive DIII-D get_n1_bradial_parameters Feb 3, 2026
@gtrevisan gtrevisan added the machine: DIII-D Related to the DIII-D tokamak label Feb 3, 2026
@gtrevisan gtrevisan marked this pull request as draft February 6, 2026 14:59
@yumouwei yumouwei force-pushed the wei/d3d-onsbradial branch from 1107210 to 19f6fd6 Compare March 11, 2026 14:46
@yumouwei yumouwei marked this pull request as ready for review April 21, 2026 15:46
Comment thread disruption_py/machine/d3d/physics.py
Comment thread disruption_py/machine/d3d/physics.py
b_tor = D3DPhysicsMethods.get_btor(params)["btor"]
if np.isnan(b_tor).all():
params.logger.warning(
"Failed to get b_tor signal to compute n_equal_1_normalized"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the name of the physics method will be attached to error log by the framework, so this might be redundant...
put an intentional error somewhere to try it out!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It looks like only the shot number is automatically tagged to the error log if I intentionally poke an error. This applies to the comment on the other method as well

08:49:57.574 [WARNING] #201927 | Failed to get b_tor signal to compute n_equal_1_normalized
08:49:57.580 [WARNING] #201927 | Failed to get b_tor signal to compute n1rms_normalized

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

because you are writing that yourself, but if you raise eg a CalculationError you will see the physics method name as well.

@gtrevisan gtrevisan requested a review from ZanderKeith April 21, 2026 16:04
@gtrevisan
Copy link
Copy Markdown
Member

@ZanderKeith, do you have some time to take a look?

b_tor, t_b_tor = params.data_conn.get_data_with_dims(
f"ptdata('bt', {params.shot_id})"
) # [ms], [T]
except mdsExceptions.MdsException as e:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure we'd need this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you mean removing this try-except block? If so then an error in this get_btor method would cause other methods that call get_btor (i.e. get_n1_bradial_parameters and get_n1rms_parameters) to raise an error and return all nans as well. I can avoid this by adding try-except blocks in those methods if that's a better way to do this.

f"ptdata('onsbradial', {params.shot_id})",
) # [G], [ms]
except mdsExceptions.MdsException:
try:
Copy link
Copy Markdown
Member

@gtrevisan gtrevisan Apr 21, 2026

Choose a reason for hiding this comment

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

this fallback would deserve at least a DEBUG statement, if not a warning!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The fallback does raise a VERBOSE statement if it successfully gets data from DUSBRADIAL. The use of VERBOSE is consistent with the fallback in CMOD get_v_loop

try:
# Fallback: get data from the legacy DUD system
n_equal_1_mode, t_n1 = params.data_conn.get_data_with_dims(
f"ptdata('dusbradial', {params.shot_id})",
) # [G], [ms]
params.logger.verbose(
"n_equal_1_mode: Failed to get ONSBRADIAL signal. Use DUSBRADIAL instead."
)

except mdsExceptions.TreeException:
params.logger.verbose(
r"v_loop: Failed to get \top.mflux:v0 data. Use \efit_aeqdsk:vloopt instead."
)
v_loop, v_loop_time = params.data_conn.get_data_with_dims(
r"\efit_aeqdsk:vloopt", tree_name="_efit_tree"
) # [V], [s]

Copy link
Copy Markdown
Member

@gtrevisan gtrevisan Apr 22, 2026

Choose a reason for hiding this comment

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

I missed that. but yeah, you should log to debug before attempting the fallback, otherwise that might never get logged.

"n_equal_1_mode: Failed to get ONSBRADIAL signal. Use DUSBRADIAL instead."
)
except mdsExceptions.MdsException:
params.logger.warning(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this could be a CalculationError which would then downplayed as a warning by the framework.

except mdsExceptions.MdsException as e:
b_tor = D3DPhysicsMethods.get_btor(params)["btor"]
if np.isnan(b_tor).all():
params.logger.warning(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you might want to raise another CalculationError, here, and remember that shot number and physics method should be tagged on by the framework!

Comment thread disruption_py/machine/d3d/physics.py
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Reintroduces DIII-D physics support for n=1 B-radial parameters by reimplementing D3DPhysicsMethods.get_n1_bradial_parameters and factoring out a reusable toroidal-field helper.

Changes:

  • Added D3DPhysicsMethods.get_btor to retrieve/interpolate toroidal field (btor).
  • Reimplemented get_n1_bradial_parameters to fetch ONFR/DUD bradial signals and compute normalized n=1 amplitude.
  • Updated get_n1rms_parameters to use get_btor for normalization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread disruption_py/machine/d3d/physics.py
Comment on lines +972 to +975
raise NotImplementedError
if 176030 <= params.shot_id <= 176912:
# TODO: load data from NetCDF file
raise NotImplementedError
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

These known shot ranges currently raise NotImplementedError. The runner treats this as an ERROR (not a WARNING), which will generate noisy error logs for an expected/known data gap. Consider returning all-NaN outputs with a warning log, or raising CalculationError with an explanatory message so it’s handled at WARNING level.

Suggested change
raise NotImplementedError
if 176030 <= params.shot_id <= 176912:
# TODO: load data from NetCDF file
raise NotImplementedError
params.logger.warning(
"n_equal_1_mode: Bradial data is unavailable for shot range "
"156199-172430; returning NaN outputs until custom tree loading "
"is implemented."
)
nan_output = np.full_like(params.times, np.nan, dtype=float)
return {
"n_equal_1_mode": nan_output,
"n_equal_1_normalized": nan_output.copy(),
}
if 176030 <= params.shot_id <= 176912:
# TODO: load data from NetCDF file
params.logger.warning(
"n_equal_1_mode: Bradial data is unavailable for shot range "
"176030-176912; returning NaN outputs until NetCDF loading "
"is implemented."
)
nan_output = np.full_like(params.times, np.nan, dtype=float)
return {
"n_equal_1_mode": nan_output,
"n_equal_1_normalized": nan_output.copy(),
}

Copilot uses AI. Check for mistakes.
Comment thread disruption_py/machine/d3d/physics.py
Comment thread disruption_py/machine/d3d/physics.py
Comment thread disruption_py/machine/d3d/physics.py
Comment thread disruption_py/machine/d3d/physics.py
Comment thread disruption_py/machine/d3d/physics.py
try:
# Fallback: get data from the legacy DUD system
n_equal_1_mode, t_n1 = params.data_conn.get_data_with_dims(
f"ptdata('dusbradial', {params.shot_id})",
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.

What is the difference between dusbradial and onsbradial? What do these signals mean / which coils are they coming from?

Towards the end of the shot they are usually in agreement but at the start they can be very different.

Copy link
Copy Markdown
Contributor

@ZanderKeith ZanderKeith left a comment

Choose a reason for hiding this comment

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

Methods look correct physics-wise. Ran them for a few shots from our recent DIII-D campaign with significant MHD activity. Signals line up well with those in reviewplus.

I'd still like to know what dusbradial vs onsbradial mean (and perhaps have a comment noting the difference), but other than that looks good to me.

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

Labels

machine: DIII-D Related to the DIII-D tokamak

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants