') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Fix inconsistent out-of-range evaluation between Tabulated1D scalar and array paths by kawacukennedy · Pull Request #4074 · openmc-dev/openmc · GitHub
Skip to content

Fix inconsistent out-of-range evaluation between Tabulated1D scalar and array paths - #4074

Open
kawacukennedy wants to merge 2 commits into
openmc-dev:developfrom
kawacukennedy:fix/issue-4041-tabulated1d-out-of-range
Open

Fix inconsistent out-of-range evaluation between Tabulated1D scalar and array paths#4074
kawacukennedy wants to merge 2 commits into
openmc-dev:developfrom
kawacukennedy:fix/issue-4041-tabulated1d-out-of-range

Conversation

@kawacukennedy

Copy link
Copy Markdown

Description

Evaluating openmc.data.Tabulated1D on values outside its tabulated range currently gives different answers depending on whether the input is a scalar or an array:

>>>tab=openmc.data.Tabulated1D([1, 2, 3], [4, 5, 6], [], [2])
>>>tab(0)
4>>>tab([0])
array([0])

The scalar path (_interpolate_scalar) returns the value at the nearest tabulated endpoint, while the array path in __call__ initializes the output with zeros and only fills points that fall inside an interpolation region, leaving out-of-range entries at zero.

This PR makes the array path assign the boundary values (y[0] / y[-1]) to out-of-range points so that both paths agree. This is also consistent with the existing precision handling at the domain edges (np.isclose checks), which already assigns endpoint values to near-edge points.

Changes

  • openmc/data/function.py:
    • Tabulated1D.__call__: out-of-range points now receive the value of the nearest tabulated endpoint, matching the scalar path.
    • Class docstring: documented the out-of-range behavior.
    • sum_functions: each tabulated component is now explicitly evaluated only where it is defined (points outside a component's own tabulated range contribute zero). This preserves the existing behavior of combined functions — e.g., FissionEnergyRelease.recoverable, total, and the q_* properties, which combine components that may cover different incident energy ranges on a union grid — independently of the new out-of-range semantics.

Testing

Added tests/unit_tests/test_function.py covering:

  • scalar/array agreement across, below, and above the tabulated range (the issue's regression case),
  • all five ENDF interpolation schemes,
  • multi-region functions,
  • exact-endpoint and floating-point-precision edge cases,
  • multidimensional input shape preservation,
  • sum_functions behavior for components with differing domains and for polynomial+tabulated combinations.

Local results: all 11 new tests pass; existing unit tests that exercise these code paths were compared before/after the change with identical outcomes (failures observed locally are due to no nuclear data being configured and are present on unmodified develop as well).

Fixes: #4041

Signed-off-by: Engineer kawacukent@gmail.com

Evaluating openmc.data.Tabulated1D on an array containing values outside
the tabulated range returned zeros for those points, while scalar
evaluation returns the value at the nearest tabulated endpoint. Assign
boundary values to out-of-range points in the array evaluation path so
that both paths agree.
sum_functions is also updated to evaluate each tabulated component only
where it is defined, which preserves the behavior of combined functions
(e.g., fission energy release components) whose tabulated components
cover different incident energy ranges.
Fixes: openmc-dev#4041
Signed-off-by: Engineer <kawacukent@gmail.com>

@CAOShurongCAOShurong 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.

Context

Reviewed exact head b39ee90 against develop base 86ceaad.

Summary

The scalar/array boundary fix is useful and the new tests cover its intended interpolation behavior. I found one blocking dtype regression in sum_functions(), however: the new accumulator inherits an integer union-grid dtype and cannot add the floating-point values returned by supported functions.

Detailed findings

Blocking issue

  • [Major] Please use an accumulator dtype compatible with the evaluated function values and add an integer-grid Tabulated1D + Polynomial regression. On the base, the supported combination returns [10.5, 20.0, 29.5] as float64; this head raises UFuncOutputCastingError while adding float64 into int64.

Verified areas

  • Purpose/scope: focused fix for #4041, with no new dependency or public API.
  • Correctness/testing: the 11 new tests pass locally; compileall and diff checking pass. The public rollup currently reports all 18 contexts successful.
  • Physics/design/performance/docs: no new physics model or transport-loop allocation; the endpoint semantics are documented and the overall design remains localized.

I used AI assistance to help inspect the repository and run the base/head verification; I checked the exact diff, reproducer, and results before submitting this review.

Comment threadopenmc/data/function.py Outdated
# Evaluate each function and add together. Tabulated functions are
# only evaluated where they are defined; values beyond a function's
# tabulated range do not contribute to the sum.
y = np.zeros_like(x)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

np.zeros_like(x) inherits x's dtype. When a tabulated grid is integer-valued, y is int64, so adding a Polynomial (or any floating-point function result) raises UFuncOutputCastingError. This worked on develop and is used by supported sum_functions() call paths. Please initialize an accumulator that can safely represent the evaluated values and cover an integer-grid Tabulated1D + Polynomial case.

np.zeros_like(x) inherits the dtype of the union grid, so when the grid is
integer-valued the accumulator cannot hold the floating-point results of
combined functions such as Polynomial, raising UFuncTypeError. Initialize
the accumulator with a float dtype, restored from the prior behavior of
sum(f(x) for f in funcs) which promoted to float.
Adds a regression test combining an integer-valued tabulated function
with a polynomial (test_sum_functions_integer_grid).
Co-authored-by: CAOShurong <notifications@github.com>
Signed-off-by: Engineer <kawacukent@gmail.com>
@kawacukennedy

Copy link
Copy Markdown
Author

Thanks @CAOShurong for the careful review and for catching the dtype regression — that is exactly right.

Reproduced: with an integer-valued grid, np.zeros_like(x) inherits int64, so adding a Polynomial result raised UFuncTypeError: Cannot cast ufunc 'add' output from dtype('float64') to dtype('int64') with casting rule 'same_kind'.

Fixed (commit 56a8e5a): the accumulator in sum_functions is now np.zeros_like(x, dtype=float), restoring the float-promoting behavior of the previous sum(f(x) for f in funcs).

Added regression test:test_sum_functions_integer_grid combines Tabulated1D([2, 4], [10, 20]) (integer grid) with Polynomial((1.0, -0.5)) and asserts the result is float64 with the expected values [10.0, 19.0] — the exact case you flagged.

The full tests/unit_tests/test_function.py suite (12 tests) passes locally. Environmental openmc.lib/nuclear-data errors observed in neighboring test files are pre-existing on unmodified develop and unrelated to these Python-only changes.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scalar v.s. array input to openmc.data.Tabulated1D gives different results.

3 participants

@kawacukennedy@CAOShurong@GuySten