wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp - #3942

Closed
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B
Closed

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp#3942
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B

Conversation

@yrrepy

@yrrepyyrrepy commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

This PR:

  1. Fixes: openmc.WeightWindowsList.from_wwinp('wwinp') fails when processing wwinp files that are larger than ~2GB
  2. Speeds-up WeightWindowsList processing (which gets to be slow with multi-GB wwinp).

  1. WeightWindowsList.export_to_hdf5 raised MemoryError on multi-GB wwinp inputs because the XML path built multi-GB ASCII strings inside lxml. Now writes HDF5 directly via h5py, matching the C++ writer.

  2. WeightWindowsList.from_wwinp was dominated by per-element isinstance checks in check_iterable_type (~90% of total time on multi-million- element bound arrays). Added a fast path for numpy float/complex ndarrays; ~11× speedup on a 172M-element wwinp (397 s → 35 s).

This enables support for many-GB wwinp files and faster processing of them.

An alternative re-factoring style is available here:
https://github.com/yrrepy/openmc/tree/wwinp_2GB_faster

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

Float/complex ndarrays are dtype-validated, so the per-element
isinstance() scan is redundant. Also construct upper_ww_bounds in
WeightWindows.__init__ as an ndarray multiplication (not a list
comprehension) so the upper-bounds setter benefits too. ~11x speedup
on 172M-element wwinp inputs (397 s -> 35 s).
@yrrepy
yrrepy requested a review from pshriwise as a code ownerMay 22, 2026 21:33
The XML serialization raised MemoryError on bound arrays >~200M
elements -- lxml's intermediate ASCII allocation fails before the
text node can be built. Write HDF5 directly via h5py, mirroring
the C++ WeightWindows::to_hdf5 writer.
Critical details for C++ compatibility:
- Bounds are 2D (ne, n_voxels) on disk (4D would segfault the
C++ tensor::Tensor<double> reader).
- max_lower_bound_ratio is written unconditionally (default 1.0).
- Root attrs filetype and version are required by
openmc_weight_windows_import.
Mesh writing is handled by a private _write_mesh_group helper in
weight_windows.py that dispatches by mesh type, matching the
reference implementation. UnstructuredMesh raises NotImplementedError
(wwinp cannot produce one).
@yrrepy
yrrepyforce-pushed the wwinp_2GB_faster_B branch from 09ed5dc to d3124eeCompareMay 23, 2026 00:15
Comment threadopenmc/weight_windows.py Outdated
Comment on lines +143 to +146
self.upper_ww_bounds = [
lb * upper_bound_ratio for lb in self.lower_ww_bounds
]
self.upper_ww_bounds = self.lower_ww_bounds * upper_bound_ratio

@GuyStenGuyStenMay 28, 2026

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.

The previous lines supported the case where self.lower_ww_bounds is a list.
The new lines do not support that.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Claude Code:
wrapped in np.asarray() so the line reads correctly even when
self.lower_ww_bounds is list-like. In practice the lower_ww_bounds setter
(L247-259) normalizes to ndarray via np.asarray, but the explicit wrap here
is clearer and costs nothing (no-op when already ndarray).

Perry:
OK, is defensive to have the explicit wrap

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.

I've missed the fact that the setter guarantees that self.lower_ww_bounds is an array. So there is no problem.
I think your original code is better.

Wrap with np.asarray() so the derivation reads correctly even when
self.lower_ww_bounds is list-like. No-op when already ndarray.
@GuySten

Copy link
Copy Markdown
Contributor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

yrrepy added 2 commits May 29, 2026 17:35
The dtype-trust fast path returned for any float/complex ndarray of
matching depth, even when expected_type was int or another class --
the docstring promised element-type validation but the fast path
skipped it. Gate the fast path on expected_type in (Real, float,
complex) so it only fires when dtype.kind in 'fc' actually satisfies
the contract.
The direct-h5py writer cannot serialize an UnstructuredMesh from pure
Python: vertex and connectivity data live in the external .exo/.h5m
file and only exist in memory after LibMesh/MOAB loads them via
openmc.lib.init. Dispatch on mesh type up front: structured meshes
take the new fast path; UnstructuredMesh falls back to the previous
TemporarySession + openmc.lib.export_weight_windows route, which also
restores honoring of init_kwargs on that path.
Removes the dead NotImplementedError branch from _write_mesh_group.
@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Further checking the code, I had unwittingly disabled a Unstructured Mesh weight window workflow

  • d5251a8 tightens the check_iterable_type fast path so it only fires when expected_type is Real/float/complex (previously any float ndarray would silently pass even when an integer type was requested).

  • 5f5ba87 restores the UnstructuredMesh path that the direct-h5py writer couldn't reach (pure Python can't materialize the external mesh data — falls back to the previous TemporarySession route, which also restores init_kwargs forwarding on that path)

Thoughts?

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

OK, I am agnostic, I just want my 2GB+ wwinps!

Do we need another opinion or should I just scrap this PR and start a new PR with the alternative approach?
Make sure it isn't disabling UM ww, etc.

@GuySten

Copy link
Copy Markdown
Contributor

I think you can just open a new PR and close this one.
You can also ask for someone else opinion if you want.

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Closed in favor of alternative approach #3951

Sign up for freeto 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

@yrrepy@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp - #3942

Closed
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B
Closed

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp#3942
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B

Conversation

@yrrepy

@yrrepyyrrepy commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

This PR:

  1. Fixes: openmc.WeightWindowsList.from_wwinp('wwinp') fails when processing wwinp files that are larger than ~2GB
  2. Speeds-up WeightWindowsList processing (which gets to be slow with multi-GB wwinp).

  1. WeightWindowsList.export_to_hdf5 raised MemoryError on multi-GB wwinp inputs because the XML path built multi-GB ASCII strings inside lxml. Now writes HDF5 directly via h5py, matching the C++ writer.

  2. WeightWindowsList.from_wwinp was dominated by per-element isinstance checks in check_iterable_type (~90% of total time on multi-million- element bound arrays). Added a fast path for numpy float/complex ndarrays; ~11× speedup on a 172M-element wwinp (397 s → 35 s).

This enables support for many-GB wwinp files and faster processing of them.

An alternative re-factoring style is available here:
https://github.com/yrrepy/openmc/tree/wwinp_2GB_faster

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

Float/complex ndarrays are dtype-validated, so the per-element
isinstance() scan is redundant. Also construct upper_ww_bounds in
WeightWindows.__init__ as an ndarray multiplication (not a list
comprehension) so the upper-bounds setter benefits too. ~11x speedup
on 172M-element wwinp inputs (397 s -> 35 s).
@yrrepy
yrrepy requested a review from pshriwise as a code ownerMay 22, 2026 21:33
The XML serialization raised MemoryError on bound arrays >~200M
elements -- lxml's intermediate ASCII allocation fails before the
text node can be built. Write HDF5 directly via h5py, mirroring
the C++ WeightWindows::to_hdf5 writer.
Critical details for C++ compatibility:
- Bounds are 2D (ne, n_voxels) on disk (4D would segfault the
C++ tensor::Tensor<double> reader).
- max_lower_bound_ratio is written unconditionally (default 1.0).
- Root attrs filetype and version are required by
openmc_weight_windows_import.
Mesh writing is handled by a private _write_mesh_group helper in
weight_windows.py that dispatches by mesh type, matching the
reference implementation. UnstructuredMesh raises NotImplementedError
(wwinp cannot produce one).
@yrrepy
yrrepyforce-pushed the wwinp_2GB_faster_B branch from 09ed5dc to d3124eeCompareMay 23, 2026 00:15
Comment threadopenmc/weight_windows.py Outdated
Comment on lines +143 to +146
self.upper_ww_bounds = [
lb * upper_bound_ratio for lb in self.lower_ww_bounds
]
self.upper_ww_bounds = self.lower_ww_bounds * upper_bound_ratio

@GuyStenGuyStenMay 28, 2026

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.

The previous lines supported the case where self.lower_ww_bounds is a list.
The new lines do not support that.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Claude Code:
wrapped in np.asarray() so the line reads correctly even when
self.lower_ww_bounds is list-like. In practice the lower_ww_bounds setter
(L247-259) normalizes to ndarray via np.asarray, but the explicit wrap here
is clearer and costs nothing (no-op when already ndarray).

Perry:
OK, is defensive to have the explicit wrap

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.

I've missed the fact that the setter guarantees that self.lower_ww_bounds is an array. So there is no problem.
I think your original code is better.

Wrap with np.asarray() so the derivation reads correctly even when
self.lower_ww_bounds is list-like. No-op when already ndarray.
@GuySten

Copy link
Copy Markdown
Contributor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

yrrepy added 2 commits May 29, 2026 17:35
The dtype-trust fast path returned for any float/complex ndarray of
matching depth, even when expected_type was int or another class --
the docstring promised element-type validation but the fast path
skipped it. Gate the fast path on expected_type in (Real, float,
complex) so it only fires when dtype.kind in 'fc' actually satisfies
the contract.
The direct-h5py writer cannot serialize an UnstructuredMesh from pure
Python: vertex and connectivity data live in the external .exo/.h5m
file and only exist in memory after LibMesh/MOAB loads them via
openmc.lib.init. Dispatch on mesh type up front: structured meshes
take the new fast path; UnstructuredMesh falls back to the previous
TemporarySession + openmc.lib.export_weight_windows route, which also
restores honoring of init_kwargs on that path.
Removes the dead NotImplementedError branch from _write_mesh_group.
@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Further checking the code, I had unwittingly disabled a Unstructured Mesh weight window workflow

  • d5251a8 tightens the check_iterable_type fast path so it only fires when expected_type is Real/float/complex (previously any float ndarray would silently pass even when an integer type was requested).

  • 5f5ba87 restores the UnstructuredMesh path that the direct-h5py writer couldn't reach (pure Python can't materialize the external mesh data — falls back to the previous TemporarySession route, which also restores init_kwargs forwarding on that path)

Thoughts?

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

OK, I am agnostic, I just want my 2GB+ wwinps!

Do we need another opinion or should I just scrap this PR and start a new PR with the alternative approach?
Make sure it isn't disabling UM ww, etc.

@GuySten

Copy link
Copy Markdown
Contributor

I think you can just open a new PR and close this one.
You can also ask for someone else opinion if you want.

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Closed in favor of alternative approach #3951

Sign up for freeto 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

@yrrepy@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp - #3942

Closed
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B
Closed

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp#3942
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B

Conversation

@yrrepy

@yrrepyyrrepy commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

This PR:

  1. Fixes: openmc.WeightWindowsList.from_wwinp('wwinp') fails when processing wwinp files that are larger than ~2GB
  2. Speeds-up WeightWindowsList processing (which gets to be slow with multi-GB wwinp).

  1. WeightWindowsList.export_to_hdf5 raised MemoryError on multi-GB wwinp inputs because the XML path built multi-GB ASCII strings inside lxml. Now writes HDF5 directly via h5py, matching the C++ writer.

  2. WeightWindowsList.from_wwinp was dominated by per-element isinstance checks in check_iterable_type (~90% of total time on multi-million- element bound arrays). Added a fast path for numpy float/complex ndarrays; ~11× speedup on a 172M-element wwinp (397 s → 35 s).

This enables support for many-GB wwinp files and faster processing of them.

An alternative re-factoring style is available here:
https://github.com/yrrepy/openmc/tree/wwinp_2GB_faster

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

Float/complex ndarrays are dtype-validated, so the per-element
isinstance() scan is redundant. Also construct upper_ww_bounds in
WeightWindows.__init__ as an ndarray multiplication (not a list
comprehension) so the upper-bounds setter benefits too. ~11x speedup
on 172M-element wwinp inputs (397 s -> 35 s).
@yrrepy
yrrepy requested a review from pshriwise as a code ownerMay 22, 2026 21:33
The XML serialization raised MemoryError on bound arrays >~200M
elements -- lxml's intermediate ASCII allocation fails before the
text node can be built. Write HDF5 directly via h5py, mirroring
the C++ WeightWindows::to_hdf5 writer.
Critical details for C++ compatibility:
- Bounds are 2D (ne, n_voxels) on disk (4D would segfault the
C++ tensor::Tensor<double> reader).
- max_lower_bound_ratio is written unconditionally (default 1.0).
- Root attrs filetype and version are required by
openmc_weight_windows_import.
Mesh writing is handled by a private _write_mesh_group helper in
weight_windows.py that dispatches by mesh type, matching the
reference implementation. UnstructuredMesh raises NotImplementedError
(wwinp cannot produce one).
@yrrepy
yrrepyforce-pushed the wwinp_2GB_faster_B branch from 09ed5dc to d3124eeCompareMay 23, 2026 00:15
Comment threadopenmc/weight_windows.py Outdated
Comment on lines +143 to +146
self.upper_ww_bounds = [
lb * upper_bound_ratio for lb in self.lower_ww_bounds
]
self.upper_ww_bounds = self.lower_ww_bounds * upper_bound_ratio

@GuyStenGuyStenMay 28, 2026

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.

The previous lines supported the case where self.lower_ww_bounds is a list.
The new lines do not support that.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Claude Code:
wrapped in np.asarray() so the line reads correctly even when
self.lower_ww_bounds is list-like. In practice the lower_ww_bounds setter
(L247-259) normalizes to ndarray via np.asarray, but the explicit wrap here
is clearer and costs nothing (no-op when already ndarray).

Perry:
OK, is defensive to have the explicit wrap

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.

I've missed the fact that the setter guarantees that self.lower_ww_bounds is an array. So there is no problem.
I think your original code is better.

Wrap with np.asarray() so the derivation reads correctly even when
self.lower_ww_bounds is list-like. No-op when already ndarray.
@GuySten

Copy link
Copy Markdown
Contributor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

yrrepy added 2 commits May 29, 2026 17:35
The dtype-trust fast path returned for any float/complex ndarray of
matching depth, even when expected_type was int or another class --
the docstring promised element-type validation but the fast path
skipped it. Gate the fast path on expected_type in (Real, float,
complex) so it only fires when dtype.kind in 'fc' actually satisfies
the contract.
The direct-h5py writer cannot serialize an UnstructuredMesh from pure
Python: vertex and connectivity data live in the external .exo/.h5m
file and only exist in memory after LibMesh/MOAB loads them via
openmc.lib.init. Dispatch on mesh type up front: structured meshes
take the new fast path; UnstructuredMesh falls back to the previous
TemporarySession + openmc.lib.export_weight_windows route, which also
restores honoring of init_kwargs on that path.
Removes the dead NotImplementedError branch from _write_mesh_group.
@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Further checking the code, I had unwittingly disabled a Unstructured Mesh weight window workflow

  • d5251a8 tightens the check_iterable_type fast path so it only fires when expected_type is Real/float/complex (previously any float ndarray would silently pass even when an integer type was requested).

  • 5f5ba87 restores the UnstructuredMesh path that the direct-h5py writer couldn't reach (pure Python can't materialize the external mesh data — falls back to the previous TemporarySession route, which also restores init_kwargs forwarding on that path)

Thoughts?

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

OK, I am agnostic, I just want my 2GB+ wwinps!

Do we need another opinion or should I just scrap this PR and start a new PR with the alternative approach?
Make sure it isn't disabling UM ww, etc.

@GuySten

Copy link
Copy Markdown
Contributor

I think you can just open a new PR and close this one.
You can also ask for someone else opinion if you want.

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Closed in favor of alternative approach #3951

Sign up for freeto 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

@yrrepy@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp - #3942

Closed
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B
Closed

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp#3942
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B

Conversation

@yrrepy

@yrrepyyrrepy commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

This PR:

  1. Fixes: openmc.WeightWindowsList.from_wwinp('wwinp') fails when processing wwinp files that are larger than ~2GB
  2. Speeds-up WeightWindowsList processing (which gets to be slow with multi-GB wwinp).

  1. WeightWindowsList.export_to_hdf5 raised MemoryError on multi-GB wwinp inputs because the XML path built multi-GB ASCII strings inside lxml. Now writes HDF5 directly via h5py, matching the C++ writer.

  2. WeightWindowsList.from_wwinp was dominated by per-element isinstance checks in check_iterable_type (~90% of total time on multi-million- element bound arrays). Added a fast path for numpy float/complex ndarrays; ~11× speedup on a 172M-element wwinp (397 s → 35 s).

This enables support for many-GB wwinp files and faster processing of them.

An alternative re-factoring style is available here:
https://github.com/yrrepy/openmc/tree/wwinp_2GB_faster

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

Float/complex ndarrays are dtype-validated, so the per-element
isinstance() scan is redundant. Also construct upper_ww_bounds in
WeightWindows.__init__ as an ndarray multiplication (not a list
comprehension) so the upper-bounds setter benefits too. ~11x speedup
on 172M-element wwinp inputs (397 s -> 35 s).
@yrrepy
yrrepy requested a review from pshriwise as a code ownerMay 22, 2026 21:33
The XML serialization raised MemoryError on bound arrays >~200M
elements -- lxml's intermediate ASCII allocation fails before the
text node can be built. Write HDF5 directly via h5py, mirroring
the C++ WeightWindows::to_hdf5 writer.
Critical details for C++ compatibility:
- Bounds are 2D (ne, n_voxels) on disk (4D would segfault the
C++ tensor::Tensor<double> reader).
- max_lower_bound_ratio is written unconditionally (default 1.0).
- Root attrs filetype and version are required by
openmc_weight_windows_import.
Mesh writing is handled by a private _write_mesh_group helper in
weight_windows.py that dispatches by mesh type, matching the
reference implementation. UnstructuredMesh raises NotImplementedError
(wwinp cannot produce one).
@yrrepy
yrrepyforce-pushed the wwinp_2GB_faster_B branch from 09ed5dc to d3124eeCompareMay 23, 2026 00:15
Comment threadopenmc/weight_windows.py Outdated
Comment on lines +143 to +146
self.upper_ww_bounds = [
lb * upper_bound_ratio for lb in self.lower_ww_bounds
]
self.upper_ww_bounds = self.lower_ww_bounds * upper_bound_ratio

@GuyStenGuyStenMay 28, 2026

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.

The previous lines supported the case where self.lower_ww_bounds is a list.
The new lines do not support that.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Claude Code:
wrapped in np.asarray() so the line reads correctly even when
self.lower_ww_bounds is list-like. In practice the lower_ww_bounds setter
(L247-259) normalizes to ndarray via np.asarray, but the explicit wrap here
is clearer and costs nothing (no-op when already ndarray).

Perry:
OK, is defensive to have the explicit wrap

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.

I've missed the fact that the setter guarantees that self.lower_ww_bounds is an array. So there is no problem.
I think your original code is better.

Wrap with np.asarray() so the derivation reads correctly even when
self.lower_ww_bounds is list-like. No-op when already ndarray.
@GuySten

Copy link
Copy Markdown
Contributor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

yrrepy added 2 commits May 29, 2026 17:35
The dtype-trust fast path returned for any float/complex ndarray of
matching depth, even when expected_type was int or another class --
the docstring promised element-type validation but the fast path
skipped it. Gate the fast path on expected_type in (Real, float,
complex) so it only fires when dtype.kind in 'fc' actually satisfies
the contract.
The direct-h5py writer cannot serialize an UnstructuredMesh from pure
Python: vertex and connectivity data live in the external .exo/.h5m
file and only exist in memory after LibMesh/MOAB loads them via
openmc.lib.init. Dispatch on mesh type up front: structured meshes
take the new fast path; UnstructuredMesh falls back to the previous
TemporarySession + openmc.lib.export_weight_windows route, which also
restores honoring of init_kwargs on that path.
Removes the dead NotImplementedError branch from _write_mesh_group.
@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Further checking the code, I had unwittingly disabled a Unstructured Mesh weight window workflow

  • d5251a8 tightens the check_iterable_type fast path so it only fires when expected_type is Real/float/complex (previously any float ndarray would silently pass even when an integer type was requested).

  • 5f5ba87 restores the UnstructuredMesh path that the direct-h5py writer couldn't reach (pure Python can't materialize the external mesh data — falls back to the previous TemporarySession route, which also restores init_kwargs forwarding on that path)

Thoughts?

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

OK, I am agnostic, I just want my 2GB+ wwinps!

Do we need another opinion or should I just scrap this PR and start a new PR with the alternative approach?
Make sure it isn't disabling UM ww, etc.

@GuySten

Copy link
Copy Markdown
Contributor

I think you can just open a new PR and close this one.
You can also ask for someone else opinion if you want.

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Closed in favor of alternative approach #3951

Sign up for freeto 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

@yrrepy@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp - #3942

Closed
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B
Closed

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp#3942
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B

Conversation

@yrrepy

@yrrepyyrrepy commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

This PR:

  1. Fixes: openmc.WeightWindowsList.from_wwinp('wwinp') fails when processing wwinp files that are larger than ~2GB
  2. Speeds-up WeightWindowsList processing (which gets to be slow with multi-GB wwinp).

  1. WeightWindowsList.export_to_hdf5 raised MemoryError on multi-GB wwinp inputs because the XML path built multi-GB ASCII strings inside lxml. Now writes HDF5 directly via h5py, matching the C++ writer.

  2. WeightWindowsList.from_wwinp was dominated by per-element isinstance checks in check_iterable_type (~90% of total time on multi-million- element bound arrays). Added a fast path for numpy float/complex ndarrays; ~11× speedup on a 172M-element wwinp (397 s → 35 s).

This enables support for many-GB wwinp files and faster processing of them.

An alternative re-factoring style is available here:
https://github.com/yrrepy/openmc/tree/wwinp_2GB_faster

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

Float/complex ndarrays are dtype-validated, so the per-element
isinstance() scan is redundant. Also construct upper_ww_bounds in
WeightWindows.__init__ as an ndarray multiplication (not a list
comprehension) so the upper-bounds setter benefits too. ~11x speedup
on 172M-element wwinp inputs (397 s -> 35 s).
@yrrepy
yrrepy requested a review from pshriwise as a code ownerMay 22, 2026 21:33
The XML serialization raised MemoryError on bound arrays >~200M
elements -- lxml's intermediate ASCII allocation fails before the
text node can be built. Write HDF5 directly via h5py, mirroring
the C++ WeightWindows::to_hdf5 writer.
Critical details for C++ compatibility:
- Bounds are 2D (ne, n_voxels) on disk (4D would segfault the
C++ tensor::Tensor<double> reader).
- max_lower_bound_ratio is written unconditionally (default 1.0).
- Root attrs filetype and version are required by
openmc_weight_windows_import.
Mesh writing is handled by a private _write_mesh_group helper in
weight_windows.py that dispatches by mesh type, matching the
reference implementation. UnstructuredMesh raises NotImplementedError
(wwinp cannot produce one).
@yrrepy
yrrepyforce-pushed the wwinp_2GB_faster_B branch from 09ed5dc to d3124eeCompareMay 23, 2026 00:15
Comment threadopenmc/weight_windows.py Outdated
Comment on lines +143 to +146
self.upper_ww_bounds = [
lb * upper_bound_ratio for lb in self.lower_ww_bounds
]
self.upper_ww_bounds = self.lower_ww_bounds * upper_bound_ratio

@GuyStenGuyStenMay 28, 2026

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.

The previous lines supported the case where self.lower_ww_bounds is a list.
The new lines do not support that.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Claude Code:
wrapped in np.asarray() so the line reads correctly even when
self.lower_ww_bounds is list-like. In practice the lower_ww_bounds setter
(L247-259) normalizes to ndarray via np.asarray, but the explicit wrap here
is clearer and costs nothing (no-op when already ndarray).

Perry:
OK, is defensive to have the explicit wrap

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.

I've missed the fact that the setter guarantees that self.lower_ww_bounds is an array. So there is no problem.
I think your original code is better.

Wrap with np.asarray() so the derivation reads correctly even when
self.lower_ww_bounds is list-like. No-op when already ndarray.
@GuySten

Copy link
Copy Markdown
Contributor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

yrrepy added 2 commits May 29, 2026 17:35
The dtype-trust fast path returned for any float/complex ndarray of
matching depth, even when expected_type was int or another class --
the docstring promised element-type validation but the fast path
skipped it. Gate the fast path on expected_type in (Real, float,
complex) so it only fires when dtype.kind in 'fc' actually satisfies
the contract.
The direct-h5py writer cannot serialize an UnstructuredMesh from pure
Python: vertex and connectivity data live in the external .exo/.h5m
file and only exist in memory after LibMesh/MOAB loads them via
openmc.lib.init. Dispatch on mesh type up front: structured meshes
take the new fast path; UnstructuredMesh falls back to the previous
TemporarySession + openmc.lib.export_weight_windows route, which also
restores honoring of init_kwargs on that path.
Removes the dead NotImplementedError branch from _write_mesh_group.
@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Further checking the code, I had unwittingly disabled a Unstructured Mesh weight window workflow

  • d5251a8 tightens the check_iterable_type fast path so it only fires when expected_type is Real/float/complex (previously any float ndarray would silently pass even when an integer type was requested).

  • 5f5ba87 restores the UnstructuredMesh path that the direct-h5py writer couldn't reach (pure Python can't materialize the external mesh data — falls back to the previous TemporarySession route, which also restores init_kwargs forwarding on that path)

Thoughts?

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

OK, I am agnostic, I just want my 2GB+ wwinps!

Do we need another opinion or should I just scrap this PR and start a new PR with the alternative approach?
Make sure it isn't disabling UM ww, etc.

@GuySten

Copy link
Copy Markdown
Contributor

I think you can just open a new PR and close this one.
You can also ask for someone else opinion if you want.

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Closed in favor of alternative approach #3951

Sign up for freeto 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

@yrrepy@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp - #3942

Closed
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B
Closed

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp#3942
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B

Conversation

@yrrepy

@yrrepyyrrepy commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

This PR:

  1. Fixes: openmc.WeightWindowsList.from_wwinp('wwinp') fails when processing wwinp files that are larger than ~2GB
  2. Speeds-up WeightWindowsList processing (which gets to be slow with multi-GB wwinp).

  1. WeightWindowsList.export_to_hdf5 raised MemoryError on multi-GB wwinp inputs because the XML path built multi-GB ASCII strings inside lxml. Now writes HDF5 directly via h5py, matching the C++ writer.

  2. WeightWindowsList.from_wwinp was dominated by per-element isinstance checks in check_iterable_type (~90% of total time on multi-million- element bound arrays). Added a fast path for numpy float/complex ndarrays; ~11× speedup on a 172M-element wwinp (397 s → 35 s).

This enables support for many-GB wwinp files and faster processing of them.

An alternative re-factoring style is available here:
https://github.com/yrrepy/openmc/tree/wwinp_2GB_faster

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

Float/complex ndarrays are dtype-validated, so the per-element
isinstance() scan is redundant. Also construct upper_ww_bounds in
WeightWindows.__init__ as an ndarray multiplication (not a list
comprehension) so the upper-bounds setter benefits too. ~11x speedup
on 172M-element wwinp inputs (397 s -> 35 s).
@yrrepy
yrrepy requested a review from pshriwise as a code ownerMay 22, 2026 21:33
The XML serialization raised MemoryError on bound arrays >~200M
elements -- lxml's intermediate ASCII allocation fails before the
text node can be built. Write HDF5 directly via h5py, mirroring
the C++ WeightWindows::to_hdf5 writer.
Critical details for C++ compatibility:
- Bounds are 2D (ne, n_voxels) on disk (4D would segfault the
C++ tensor::Tensor<double> reader).
- max_lower_bound_ratio is written unconditionally (default 1.0).
- Root attrs filetype and version are required by
openmc_weight_windows_import.
Mesh writing is handled by a private _write_mesh_group helper in
weight_windows.py that dispatches by mesh type, matching the
reference implementation. UnstructuredMesh raises NotImplementedError
(wwinp cannot produce one).
@yrrepy
yrrepyforce-pushed the wwinp_2GB_faster_B branch from 09ed5dc to d3124eeCompareMay 23, 2026 00:15
Comment threadopenmc/weight_windows.py Outdated
Comment on lines +143 to +146
self.upper_ww_bounds = [
lb * upper_bound_ratio for lb in self.lower_ww_bounds
]
self.upper_ww_bounds = self.lower_ww_bounds * upper_bound_ratio

@GuyStenGuyStenMay 28, 2026

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.

The previous lines supported the case where self.lower_ww_bounds is a list.
The new lines do not support that.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Claude Code:
wrapped in np.asarray() so the line reads correctly even when
self.lower_ww_bounds is list-like. In practice the lower_ww_bounds setter
(L247-259) normalizes to ndarray via np.asarray, but the explicit wrap here
is clearer and costs nothing (no-op when already ndarray).

Perry:
OK, is defensive to have the explicit wrap

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.

I've missed the fact that the setter guarantees that self.lower_ww_bounds is an array. So there is no problem.
I think your original code is better.

Wrap with np.asarray() so the derivation reads correctly even when
self.lower_ww_bounds is list-like. No-op when already ndarray.
@GuySten

Copy link
Copy Markdown
Contributor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

yrrepy added 2 commits May 29, 2026 17:35
The dtype-trust fast path returned for any float/complex ndarray of
matching depth, even when expected_type was int or another class --
the docstring promised element-type validation but the fast path
skipped it. Gate the fast path on expected_type in (Real, float,
complex) so it only fires when dtype.kind in 'fc' actually satisfies
the contract.
The direct-h5py writer cannot serialize an UnstructuredMesh from pure
Python: vertex and connectivity data live in the external .exo/.h5m
file and only exist in memory after LibMesh/MOAB loads them via
openmc.lib.init. Dispatch on mesh type up front: structured meshes
take the new fast path; UnstructuredMesh falls back to the previous
TemporarySession + openmc.lib.export_weight_windows route, which also
restores honoring of init_kwargs on that path.
Removes the dead NotImplementedError branch from _write_mesh_group.
@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Further checking the code, I had unwittingly disabled a Unstructured Mesh weight window workflow

  • d5251a8 tightens the check_iterable_type fast path so it only fires when expected_type is Real/float/complex (previously any float ndarray would silently pass even when an integer type was requested).

  • 5f5ba87 restores the UnstructuredMesh path that the direct-h5py writer couldn't reach (pure Python can't materialize the external mesh data — falls back to the previous TemporarySession route, which also restores init_kwargs forwarding on that path)

Thoughts?

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

OK, I am agnostic, I just want my 2GB+ wwinps!

Do we need another opinion or should I just scrap this PR and start a new PR with the alternative approach?
Make sure it isn't disabling UM ww, etc.

@GuySten

Copy link
Copy Markdown
Contributor

I think you can just open a new PR and close this one.
You can also ask for someone else opinion if you want.

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Closed in favor of alternative approach #3951

Sign up for freeto 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

@yrrepy@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp - #3942

Closed
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B
Closed

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp#3942
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B

Conversation

@yrrepy

@yrrepyyrrepy commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

This PR:

  1. Fixes: openmc.WeightWindowsList.from_wwinp('wwinp') fails when processing wwinp files that are larger than ~2GB
  2. Speeds-up WeightWindowsList processing (which gets to be slow with multi-GB wwinp).

  1. WeightWindowsList.export_to_hdf5 raised MemoryError on multi-GB wwinp inputs because the XML path built multi-GB ASCII strings inside lxml. Now writes HDF5 directly via h5py, matching the C++ writer.

  2. WeightWindowsList.from_wwinp was dominated by per-element isinstance checks in check_iterable_type (~90% of total time on multi-million- element bound arrays). Added a fast path for numpy float/complex ndarrays; ~11× speedup on a 172M-element wwinp (397 s → 35 s).

This enables support for many-GB wwinp files and faster processing of them.

An alternative re-factoring style is available here:
https://github.com/yrrepy/openmc/tree/wwinp_2GB_faster

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

Float/complex ndarrays are dtype-validated, so the per-element
isinstance() scan is redundant. Also construct upper_ww_bounds in
WeightWindows.__init__ as an ndarray multiplication (not a list
comprehension) so the upper-bounds setter benefits too. ~11x speedup
on 172M-element wwinp inputs (397 s -> 35 s).
@yrrepy
yrrepy requested a review from pshriwise as a code ownerMay 22, 2026 21:33
The XML serialization raised MemoryError on bound arrays >~200M
elements -- lxml's intermediate ASCII allocation fails before the
text node can be built. Write HDF5 directly via h5py, mirroring
the C++ WeightWindows::to_hdf5 writer.
Critical details for C++ compatibility:
- Bounds are 2D (ne, n_voxels) on disk (4D would segfault the
C++ tensor::Tensor<double> reader).
- max_lower_bound_ratio is written unconditionally (default 1.0).
- Root attrs filetype and version are required by
openmc_weight_windows_import.
Mesh writing is handled by a private _write_mesh_group helper in
weight_windows.py that dispatches by mesh type, matching the
reference implementation. UnstructuredMesh raises NotImplementedError
(wwinp cannot produce one).
@yrrepy
yrrepyforce-pushed the wwinp_2GB_faster_B branch from 09ed5dc to d3124eeCompareMay 23, 2026 00:15
Comment threadopenmc/weight_windows.py Outdated
Comment on lines +143 to +146
self.upper_ww_bounds = [
lb * upper_bound_ratio for lb in self.lower_ww_bounds
]
self.upper_ww_bounds = self.lower_ww_bounds * upper_bound_ratio

@GuyStenGuyStenMay 28, 2026

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.

The previous lines supported the case where self.lower_ww_bounds is a list.
The new lines do not support that.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Claude Code:
wrapped in np.asarray() so the line reads correctly even when
self.lower_ww_bounds is list-like. In practice the lower_ww_bounds setter
(L247-259) normalizes to ndarray via np.asarray, but the explicit wrap here
is clearer and costs nothing (no-op when already ndarray).

Perry:
OK, is defensive to have the explicit wrap

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.

I've missed the fact that the setter guarantees that self.lower_ww_bounds is an array. So there is no problem.
I think your original code is better.

Wrap with np.asarray() so the derivation reads correctly even when
self.lower_ww_bounds is list-like. No-op when already ndarray.
@GuySten

Copy link
Copy Markdown
Contributor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

yrrepy added 2 commits May 29, 2026 17:35
The dtype-trust fast path returned for any float/complex ndarray of
matching depth, even when expected_type was int or another class --
the docstring promised element-type validation but the fast path
skipped it. Gate the fast path on expected_type in (Real, float,
complex) so it only fires when dtype.kind in 'fc' actually satisfies
the contract.
The direct-h5py writer cannot serialize an UnstructuredMesh from pure
Python: vertex and connectivity data live in the external .exo/.h5m
file and only exist in memory after LibMesh/MOAB loads them via
openmc.lib.init. Dispatch on mesh type up front: structured meshes
take the new fast path; UnstructuredMesh falls back to the previous
TemporarySession + openmc.lib.export_weight_windows route, which also
restores honoring of init_kwargs on that path.
Removes the dead NotImplementedError branch from _write_mesh_group.
@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Further checking the code, I had unwittingly disabled a Unstructured Mesh weight window workflow

  • d5251a8 tightens the check_iterable_type fast path so it only fires when expected_type is Real/float/complex (previously any float ndarray would silently pass even when an integer type was requested).

  • 5f5ba87 restores the UnstructuredMesh path that the direct-h5py writer couldn't reach (pure Python can't materialize the external mesh data — falls back to the previous TemporarySession route, which also restores init_kwargs forwarding on that path)

Thoughts?

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

OK, I am agnostic, I just want my 2GB+ wwinps!

Do we need another opinion or should I just scrap this PR and start a new PR with the alternative approach?
Make sure it isn't disabling UM ww, etc.

@GuySten

Copy link
Copy Markdown
Contributor

I think you can just open a new PR and close this one.
You can also ask for someone else opinion if you want.

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Closed in favor of alternative approach #3951

Sign up for freeto 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

@yrrepy@GuySten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp - #3942

Closed
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B
Closed

wwinp files: Fix MemoryError in WeightWindowsList.export_to_hdf5 and speed up from_wwinp#3942
yrrepy wants to merge 5 commits into
openmc-dev:developfrom
yrrepy:wwinp_2GB_faster_B

Conversation

@yrrepy

@yrrepyyrrepy commented May 22, 2026

Copy link
Copy Markdown
Contributor

Description

This PR:

  1. Fixes: openmc.WeightWindowsList.from_wwinp('wwinp') fails when processing wwinp files that are larger than ~2GB
  2. Speeds-up WeightWindowsList processing (which gets to be slow with multi-GB wwinp).

  1. WeightWindowsList.export_to_hdf5 raised MemoryError on multi-GB wwinp inputs because the XML path built multi-GB ASCII strings inside lxml. Now writes HDF5 directly via h5py, matching the C++ writer.

  2. WeightWindowsList.from_wwinp was dominated by per-element isinstance checks in check_iterable_type (~90% of total time on multi-million- element bound arrays). Added a fast path for numpy float/complex ndarrays; ~11× speedup on a 172M-element wwinp (397 s → 35 s).

This enables support for many-GB wwinp files and faster processing of them.

An alternative re-factoring style is available here:
https://github.com/yrrepy/openmc/tree/wwinp_2GB_faster

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

Float/complex ndarrays are dtype-validated, so the per-element
isinstance() scan is redundant. Also construct upper_ww_bounds in
WeightWindows.__init__ as an ndarray multiplication (not a list
comprehension) so the upper-bounds setter benefits too. ~11x speedup
on 172M-element wwinp inputs (397 s -> 35 s).
@yrrepy
yrrepy requested a review from pshriwise as a code ownerMay 22, 2026 21:33
The XML serialization raised MemoryError on bound arrays >~200M
elements -- lxml's intermediate ASCII allocation fails before the
text node can be built. Write HDF5 directly via h5py, mirroring
the C++ WeightWindows::to_hdf5 writer.
Critical details for C++ compatibility:
- Bounds are 2D (ne, n_voxels) on disk (4D would segfault the
C++ tensor::Tensor<double> reader).
- max_lower_bound_ratio is written unconditionally (default 1.0).
- Root attrs filetype and version are required by
openmc_weight_windows_import.
Mesh writing is handled by a private _write_mesh_group helper in
weight_windows.py that dispatches by mesh type, matching the
reference implementation. UnstructuredMesh raises NotImplementedError
(wwinp cannot produce one).
@yrrepy
yrrepyforce-pushed the wwinp_2GB_faster_B branch from 09ed5dc to d3124eeCompareMay 23, 2026 00:15
Comment threadopenmc/weight_windows.py Outdated
Comment on lines +143 to +146
self.upper_ww_bounds = [
lb * upper_bound_ratio for lb in self.lower_ww_bounds
]
self.upper_ww_bounds = self.lower_ww_bounds * upper_bound_ratio

@GuyStenGuyStenMay 28, 2026

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.

The previous lines supported the case where self.lower_ww_bounds is a list.
The new lines do not support that.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Claude Code:
wrapped in np.asarray() so the line reads correctly even when
self.lower_ww_bounds is list-like. In practice the lower_ww_bounds setter
(L247-259) normalizes to ndarray via np.asarray, but the explicit wrap here
is clearer and costs nothing (no-op when already ndarray).

Perry:
OK, is defensive to have the explicit wrap

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.

I've missed the fact that the setter guarantees that self.lower_ww_bounds is an array. So there is no problem.
I think your original code is better.

Wrap with np.asarray() so the derivation reads correctly even when
self.lower_ww_bounds is list-like. No-op when already ndarray.
@GuySten

Copy link
Copy Markdown
Contributor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

yrrepy added 2 commits May 29, 2026 17:35
The dtype-trust fast path returned for any float/complex ndarray of
matching depth, even when expected_type was int or another class --
the docstring promised element-type validation but the fast path
skipped it. Gate the fast path on expected_type in (Real, float,
complex) so it only fires when dtype.kind in 'fc' actually satisfies
the contract.
The direct-h5py writer cannot serialize an UnstructuredMesh from pure
Python: vertex and connectivity data live in the external .exo/.h5m
file and only exist in memory after LibMesh/MOAB loads them via
openmc.lib.init. Dispatch on mesh type up front: structured meshes
take the new fast path; UnstructuredMesh falls back to the previous
TemporarySession + openmc.lib.export_weight_windows route, which also
restores honoring of init_kwargs on that path.
Removes the dead NotImplementedError branch from _write_mesh_group.
@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Further checking the code, I had unwittingly disabled a Unstructured Mesh weight window workflow

  • d5251a8 tightens the check_iterable_type fast path so it only fires when expected_type is Real/float/complex (previously any float ndarray would silently pass even when an integer type was requested).

  • 5f5ba87 restores the UnstructuredMesh path that the direct-h5py writer couldn't reach (pure Python can't materialize the external mesh data — falls back to the previous TemporarySession route, which also restores init_kwargs forwarding on that path)

Thoughts?

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

IMO the alternative refactoring style is better. It will be easier to support new mesh types when each mesh manage for itself the serialization to hdf5.

OK, I am agnostic, I just want my 2GB+ wwinps!

Do we need another opinion or should I just scrap this PR and start a new PR with the alternative approach?
Make sure it isn't disabling UM ww, etc.

@GuySten

Copy link
Copy Markdown
Contributor

I think you can just open a new PR and close this one.
You can also ask for someone else opinion if you want.

@yrrepy

Copy link
Copy Markdown
ContributorAuthor

Closed in favor of alternative approach #3951

Sign up for freeto 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

@yrrepy@GuySten