Skip to content

BUG: Make preflight geometry import metadata-only#1667

Merged
imikejackson merged 2 commits into
BlueQuartzSoftware:developfrom
joeykleingers:topic/metadata-only-preflight
Jul 10, 2026
Merged

BUG: Make preflight geometry import metadata-only#1667
imikejackson merged 2 commits into
BlueQuartzSoftware:developfrom
joeykleingers:topic/metadata-only-preflight

Conversation

@joeykleingers

@joeykleingers joeykleingers commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

During a metadata-only preflight, DataStructureReader now leaves geometry connectivity — vertex/face/edge lists, derived connectivity, and rectilinear-grid bounds — as an EmptyDataStore carrying only shape and type (exactly like ordinary attribute arrays), instead of eagerly bulk-reading it from disk. This removes a wasted per-geometry disk read from every preflight, which is costly on high-latency storage such as network mounts. Execute is unchanged — it imports the real connectivity stores directly in readGroup.

  • readGroup already imports every array as an EmptyDataStore when useEmptyDataStores is set, so loadRequiredData is now skipped during preflight.
  • Geometry IO readers (EdgeGeomIO, INodeGeom0d/1d/2d/3dIO, RectGridGeomIO) no longer register connectivity as required data — those ids were only ever consumed by the eager preflight read.
  • Forward useEmptyDataStores from the std::filesystem::path overload of DataStructureReader::ReadFile to the FileIO overload; it was previously dropped and always defaulted to false.

Test Plan

  • DREAM3DFileTest passes (adds preflight metadata-only coverage asserting geometry connectivity stores are empty after preflight and populated after execute)
  • Existing DREAM3D read/write and geometry tests pass

@joeykleingers joeykleingers added the bug Something isn't working label Jul 9, 2026
@joeykleingers joeykleingers changed the title ENH: Make preflight geometry import metadata-only BUG: Make preflight geometry import metadata-only Jul 9, 2026
@imikejackson

Copy link
Copy Markdown
Contributor

I pushed a3fcfc7 to this branch after reviewing — two things, no change to the approach:

  • Removed the now-dead required-data machinery from DataStructureReader. With the geometry IO readers no longer registering connectivity ids, nothing calls addRequiredId/addRequiredPath anymore, so loadRequiredData was a guaranteed no-op in every code path — including the !useEmptyDataStores call this PR kept, which implied the pass still mattered for full reads. Deleted addRequiredPath, both addRequiredId overloads, loadRequiredData, and the m_RequiredPaths/m_RequiredIds members. The static FinishImportingObject stays; Dream3dIO uses it for the per-path execute import.

  • Broadened the new test to cover every reader this PR touched. It previously only exercised TriangleGeometry (INodeGeom2dIO); it's now table-driven over geoms.dream3d's edge, triangle, quad, tetrahedral, and hexahedral geometries plus the rect-grid X/Y/Z bounds, with a DYNAMIC_SECTION per connectivity array in all three blocks (preflight, execute, ReadFile path overload).

Also chased the two riskiest paths and found no regressions: no preflightImpl in simplnx or downstream plugins reads connectivity values (only shape/type, which EmptyDataStore preserves — value access throws loudly), and execute through ImportH5ObjectPathsAction is unaffected because connectivity arrays are ordinary paths in m_Paths and get their real stores from DataArrayIO::finishImportingData.

Verified on both builds: full SimplnxCore + core-library suites pass 100% on in-core (simplnx-Rel) and out-of-core (simplnx-ooc-Rel).

@imikejackson imikejackson force-pushed the topic/metadata-only-preflight branch from a3fcfc7 to ffd6a6e Compare July 10, 2026 17:57
joeykleingers and others added 2 commits July 10, 2026 14:43
During a metadata-only preflight, DataStructureReader now leaves geometry
connectivity -- vertex/face/edge lists, derived connectivity, and
rectilinear-grid bounds -- as an EmptyDataStore that carries only shape and
type, exactly like ordinary attribute arrays, instead of eagerly bulk-reading
it from disk. readGroup already imports every array as an EmptyDataStore when
useEmptyDataStores is set, so loadRequiredData is now skipped during preflight
and the geometry IO readers no longer register connectivity as required data
(those ids were only ever consumed by the eager preflight read). This removes a
wasted per-geometry disk read from every preflight, which is costly on
high-latency storage. Execute is unchanged: it imports the real connectivity
stores directly in readGroup.

Also forward useEmptyDataStores from the std::filesystem::path overload of
DataStructureReader::ReadFile to the FileIO overload; it was previously dropped
and always defaulted to false.
… test

* With the geometry IO readers no longer registering connectivity ids,
  nothing ever calls addRequiredId/addRequiredPath, making
  loadRequiredData a guaranteed no-op in every code path. Remove the
  entire mechanism (addRequiredPath, both addRequiredId overloads,
  loadRequiredData, m_RequiredPaths, m_RequiredIds) instead of leaving
  a call site that implies the pass still matters for full reads.
* Broaden the metadata-only preflight test to cover every reader the
  change touched: edge, triangle, quad, tetrahedral, and hexahedral
  geometries plus rectilinear-grid X/Y/Z bounds, table-driven with
  DYNAMIC_SECTION per connectivity array.

Signed-off-by: Michael Jackson <mike.jackson@bluequartz.net>
@imikejackson imikejackson force-pushed the topic/metadata-only-preflight branch from ffd6a6e to 452e198 Compare July 10, 2026 18:43
@imikejackson imikejackson merged commit 6224049 into BlueQuartzSoftware:develop Jul 10, 2026
6 checks passed
@imikejackson imikejackson deleted the topic/metadata-only-preflight branch July 10, 2026 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants