Skip to content

Fix profile extraction: stop merging volume data, initialise all ProfileData fields - #222

Closed
mthielma with Copilot wants to merge 2 commits into
profile_processing_mtfrom
copilot/profile-processing-mt
Closed

mthielma with Copilot wants to merge 2 commits into
profile_processing_mtfrom
copilot/profile-processing-mt

Conversation

Copilot AI commented Oct 2, 2026 •

Copy link
Copy Markdown

Profile processing should take volume datasets as a named tuple and not merge them into one grid first, which is slow when there are many volume datasets. It also needs to pass topography data through and keep ProfileData.TopoData usable.

Most of this was already in place (TopoData field, named-tuple create_profile_volume!, topo arguments on extract_ProfileData!). Two gaps remained:

  • ProfileData constructor

    • new(...) was given 7 values for a 9-field struct, so TopoData, PointData and ScreenshotData were left undefined on a fresh profile.
    • It now initialises all 9 fields to nothing.
    • Without this, reading profile.TopoData or show(profile) fails on an unset field.
  • extract_ProfileData(file, n, datasetfile) convenience wrapper

    • It no longer calls combine_vol_data.
    • It passes the NamedTuple of volume datasets straight to extract_ProfileData!, so each dataset is cross-sectioned on its own.
    • It now also forwards TopoData and ScreenshotData, which load_GMG already returned but the wrapper dropped.
# before: merge every volume dataset onto one grid, then slice
extract_ProfileData!(profile, combine_vol_data(VolData), SurfData, PointData; ...)

# after: slice each volume dataset directly, and include topography
extract_ProfileData!(profile, VolData, SurfData, PointData; TopoData = TopoData, ScreenshotData = ScreenshotData, ...)

Notes for review

  • The package could not be loaded in my environment (dependencies not installed), so none of this has been run. The test file also downloads its datasets over the network.
  • The new test only checks that TopoData is empty on a fresh ProfileData. Topography extraction itself is not covered.
  • Some existing calls in test/test_ProfileProcessing.jl may not match any current method. They pass nothing as the volume data, and pass screenshot data as a fifth positional argument. I left them unchanged.

@mthielma
mthielma added this pull request to stack #223 October 2, 2026 11:19
…ata, fix ProfileData constructor

Co-authored-by: mthielma <1148509+mthielma@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement profile processing improvements as per PR #215 Fix profile extraction: stop merging volume data, initialise all ProfileData fields Oct 2, 2026
Copilot AI requested a review from mthielma October 2, 2026 11:21
@mthielma

mthielma commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Copilot went a bit too far here, as it started to fix all kinds of issues, not only the one I asked for. I am therefore also closing this one.

@mthielma mthielma closed this Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants