Feature:`gia2plmt`-SELEN compatibility by williameclee · Pull Request #33 · williameclee/ulmo

williameclee · 2026-04-01T18:25:35Z

No description provided.

…hy repo

Copilot

Pull request overview

This PR adds MATLAB utilities to improve compatibility with SELEN model outputs and to standardize longitude/latitude input parsing and axis tick formatting across plotting helpers.

Changes:

Added convertSelenModel to convert SELEN gdot.pix/udot.pix outputs into SLEPIAN-style spherical harmonic .mat files.
Added parselonlatinputs to accept lon/lat pairs, lonlat matrices, polyshape, or GeoDomain inputs and return normalized lon/lat plus remaining arguments.
Added formatlonticks / formatlatticks to format axes tick labels as geographic coordinates.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.

File	Description
`plotting/parselonlatinputs.m`	New helper to normalize lon/lat-like inputs for plotting/geometry utilities.
`plotting/formatlonticks.m`	New helper to format x-axis ticks as longitudes.
`plotting/formatlatticks.m`	New helper to format y-axis ticks as latitudes.
`dataio/convertSelenModel.m`	New converter from SELEN output grids to SLEPIAN-compatible coefficient files.

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

Copilot

Pull request overview

Copilot reviewed 11 out of 24 changed files in this pull request and generated 7 comments.

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

Copilot · 2026-04-07T18:54:55Z

+    if isempty(needsAnchor)
+
+        if nargout == 1
+
+            if isPoly
+                varargout = {p};
+            else
+                varargout = {[lon, lat]};
+            end


When needsAnchor is empty and the input was a polyshape, this path returns {p} but p is undefined (it was overwritten by vertices). This will throw even though no anchors are needed. Consider returning the original polyshape (store it before overwriting), or reconstruct it from lon/lat (e.g., polyshape(lon, lat, ...)).

Copilot · 2026-04-07T18:54:55Z

    if ~beQuiet
        wbar = waitbar(0, 'Initialising', ...
            "Name", upper(mfilename), "CreateCancelBtn", 'setappdata(gcbf,''canceling'',1)');
+        cleanup = onCleanup(@() delete(wbar));


The added onCleanup(@() delete(wbar)) will run even if wbar is deleted manually later (this function calls delete(wbar) in several places). That can lead to a double-delete and an error on function exit. Consider making the cleanup callback resilient (e.g., check ishghandle/isvalid before deleting), or stop manually deleting wbar and rely on the cleanup (or clear the cleanup handle after manual delete).

Suggested change

cleanup = onCleanup(@() delete(wbar));

cleanup = onCleanup(@() (ishghandle(wbar) && delete(wbar)));

Copilot · 2026-04-07T18:54:56Z

    %% Loading data
    wbar = waitbar(0, 'Loading GRACE data', ...
        "Name", upper(mfilename), "CreateCancelBtn", 'setappdata(gcbf,''canceling'',1)');
+    cleanup = onCleanup(@() delete(wbar));



Same issue as in solvesle: onCleanup(@() delete(wbar)) is added, but the function still manually calls delete(wbar) on cancellation and at the end. This can double-delete the handle and error during cleanup. Make the cleanup callback safe (check handle validity) or remove/replace the manual deletes.

Copilot · 2026-04-07T18:54:56Z

+    modelName = regexp(modelFolderSelf, "RUN_([A-Za-z0-9-_]+)", "tokens", "once");
+    modelInfo = regexp(modelFolderSelf, "RUN_([A-Za-z0-9]+)-R(\d+)-L(\d+)-I(\d+)", "tokens", "once");
+
+    % Prepare grid for interpolation
+    L = str2double(modelInfo{3});


modelName is extracted via regexp(...,'tokens','once'), which returns a cell array; passing that directly to sprintf('%s_SD.mat', modelName) will error. Also, modelInfo is parsed with a pattern that doesn’t allow dashes/underscores in the model name; if the folder name includes them (allowed by the modelName regex), modelInfo will be empty and modelInfo{3} will error. Consider normalising to string/char (modelName = string(modelName{1})) and validating modelInfo before indexing (or use a single regex that captures name and L robustly).

Suggested change

modelName = regexp(modelFolderSelf, "RUN_([A-Za-z0-9-_]+)", "tokens", "once");

modelInfo = regexp(modelFolderSelf, "RUN_([A-Za-z0-9]+)-R(\d+)-L(\d+)-I(\d+)", "tokens", "once");

% Prepare grid for interpolation

L = str2double(modelInfo{3});

modelInfo = regexp(modelFolderSelf, "^RUN_([A-Za-z0-9_-]+)-R(\d+)-L(\d+)-I(\d+)$", "tokens", "once");

if isempty(modelInfo)

error("convertSelenModel:InvalidModelFolderName", ...

"Model folder name '%s' does not match expected pattern RUN_<name>-R<number>-L<number>-I<number>.", ...

modelFolderSelf);

end

modelName = modelInfo{1};

% Prepare grid for interpolation

L = str2double(modelInfo{3});

if isnan(L)

error("convertSelenModel:InvalidModelDegree", ...

"Unable to parse spherical harmonic degree L from model folder name '%s'.", ...

modelFolderSelf);

end

Copilot · 2026-04-07T18:54:56Z

+    if ~isempty(newModel)
+        newModel = sprintf("%s-%s_V%s_T%s", newModel, upper(icesheet), ...
+            replace(replace(sprintf("%+03.0f", volumeChangeFrac * 100), "+", "p"), "-", "n"), ...
+            replace(replace(sprintf("%+03d", timeDelay), "+", "p"), "-", "n"));
+    end
+


The newModel naming logic appears inverted: the doc says an empty newModel should trigger auto-generation, but the code only modifies newModel when it is not empty. As written, leaving newModel empty will produce newModelPath pointing at the SELEN/DATA folder (and fopen will fail). Also, newModelPath does not append the .pix extension even though the input model is .pix. Consider generating a default filename when newModel is empty and ensuring the output path ends with .pix.

Suggested change

if ~isempty(newModel)

newModel = sprintf("%s-%s_V%s_T%s", newModel, upper(icesheet), ...

replace(replace(sprintf("%+03.0f", volumeChangeFrac * 100), "+", "p"), "-", "n"), ...

replace(replace(sprintf("%+03d", timeDelay), "+", "p"), "-", "n"));

end

newModelBase = char(newModel);

if isempty(newModelBase)

newModelBase = char(model);

end

newModel = sprintf("%s-%s_V%s_T%s", newModelBase, upper(icesheet), ...

replace(replace(sprintf("%+03.0f", volumeChangeFrac * 100), "+", "p"), "-", "n"), ...

replace(replace(sprintf("%+03d", timeDelay), "+", "p"), "-", "n"));

if ~endsWith(newModel, ".pix")

newModel = strcat(newModel, ".pix");

end

Copilot · 2026-04-07T18:54:57Z

@@ -0,0 +1,55 @@
+%% ICESHEETPOLY - Gets approximate


The header line is incomplete ("Gets approximate") and doesn’t describe what the function returns (ice sheet polygon). Consider updating the first doc line to a complete summary consistent with other functions’ headers.

Suggested change

%% ICESHEETPOLY - Gets approximate

%% ICESHEETPOLY - Gets an approximate ice sheet polygon

williameclee added 2 commits April 1, 2026 11:12

Port: Ported plotting aux functions back from the numerical cartograp…

632d6fa

…hy repo

Feature: Added compatibility with SELEN model outputs

7a9ddae

williameclee requested a review from Copilot April 1, 2026 18:25

williameclee self-assigned this Apr 1, 2026

williameclee added the feature New feature or request label Apr 1, 2026

williameclee marked this pull request as draft April 1, 2026 18:25

Copilot started reviewing on behalf of williameclee April 1, 2026 18:26 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

williameclee added 8 commits April 1, 2026 15:22

Doc: Fixed docstring inconsistencies

eb08484

Refactor: Moved redundant code and moved blocks around for readability

f0ad224

Fix: Made sure the waitbar is properly deleted

e64fc98

Refactor: Renamed domain folder

c3ffc95

Refactor: Renamed folder of aux functions for SELEN

7e394f2

Feature: Added ability to modify ice models from SELEN

3ee6369

Feature: Added support for loading GIA models from SELEN

773448f

Port: Ported from the numerical cartography project

41dcf18

williameclee requested a review from Copilot April 7, 2026 18:49

Copilot started reviewing on behalf of williameclee April 7, 2026 18:50 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

williameclee added 2 commits April 8, 2026 14:54

Merge branch 'feature/selen' into feature/gia2plmt-selen-compatibility

41f7672

Doc: Fixed docstring title

baa7407

	cleanup = onCleanup(@() delete(wbar));
	cleanup = onCleanup(@() (ishghandle(wbar) && delete(wbar)));

-    modelName = regexp(modelFolderSelf, "RUN_([A-Za-z0-9-_]+)", "tokens", "once");
-    modelInfo = regexp(modelFolderSelf, "RUN_([A-Za-z0-9]+)-R(\d+)-L(\d+)-I(\d+)", "tokens", "once");
-    % Prepare grid for interpolation
-    L = str2double(modelInfo{3});
+    modelInfo = regexp(modelFolderSelf, "^RUN_([A-Za-z0-9_-]+)-R(\d+)-L(\d+)-I(\d+)$", "tokens", "once");
+    if isempty(modelInfo)
+        error("convertSelenModel:InvalidModelFolderName", ...
+            "Model folder name '%s' does not match expected pattern RUN_<name>-R<number>-L<number>-I<number>.", ...
+            modelFolderSelf);
+    end
+    modelName = modelInfo{1};
+    % Prepare grid for interpolation
+    L = str2double(modelInfo{3});
+    if isnan(L)
+        error("convertSelenModel:InvalidModelDegree", ...
+            "Unable to parse spherical harmonic degree L from model folder name '%s'.", ...
+            modelFolderSelf);
+    end

	%% ICESHEETPOLY - Gets approximate
	%% ICESHEETPOLY - Gets an approximate ice sheet polygon

Conversation

williameclee commented Apr 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants