Skip to content

Conversation

@djmaxus
Copy link
Collaborator

@djmaxus djmaxus commented Nov 19, 2025

Closes #156

@djmaxus djmaxus self-assigned this Nov 19, 2025
Copilot AI review requested due to automatic review settings November 19, 2025 11:37
@djmaxus djmaxus added bug Something isn't working feature Adding a new functionality, small or large labels Nov 19, 2025

This comment was marked as outdated.

Instead, just use capillary pressure data from KRNUM.

Move `write_mappings` to a new directory with common export functions.
Splits export functions into common and PFLOTRAN-OGS groups.

Organizes export-related utilities by separating shared and
simulator-specific logic, improving code clarity and maintainability.
@djmaxus djmaxus linked an issue Nov 19, 2025 that may be closed by this pull request
@djmaxus djmaxus requested a review from Copilot November 19, 2025 12:07

This comment was marked as outdated.

@djmaxus djmaxus requested a review from Copilot November 19, 2025 12:24
@djmaxus djmaxus merged commit 5d840f7 into main Nov 19, 2025
15 checks passed
@djmaxus djmaxus deleted the export-opm-flow branch November 19, 2025 12:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 8 out of 15 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

src/export/common/write_mappings.m:1

  • The function lacks documentation. Consider adding a help comment block describing:
  • Purpose of the function
  • What each parameter represents
  • What files are generated
  • Example usage

This would improve maintainability and help users understand how to use the function.


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

"INCLUDE"
"JFUNC.inc /"
};
grid_fid = fopen([output_prefix,'GRID.inc'],'wb','native','UTF-8');
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

No error handling is performed after fopen. If the file cannot be opened, grid_fid will be -1, and subsequent operations will fail silently. Consider adding error checking or using a helper function with proper error handling.

Copilot uses AI. Check for mistakes.
strata_trapped.porosity,...
strata_trapped.permeability);

sgwfn_fid = fopen([prefix,'SGWFN.inc'],'wb','native','UTF-8');
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

No error handling is performed after fopen. If the file cannot be opened, sgwfn_fid will be -1, and subsequent operations will fail silently. Consider adding error checking or using a helper function with proper error handling.

Suggested change
sgwfn_fid = fopen([prefix,'SGWFN.inc'],'wb','native','UTF-8');
sgwfn_fid = fopen([prefix,'SGWFN.inc'],'wb','native','UTF-8');
if sgwfn_fid == -1
error('Failed to open file for writing: %sSGWFN.inc', prefix);
end

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +5
function [] = opm_export(strata_trapped, output_folder)
arguments
strata_trapped (1,1) struct
output_folder char = 'opm'
end
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The function lacks documentation. Consider adding a help comment block describing:

  • Purpose of the function
  • Expected structure/fields of strata_trapped parameter
  • Description of the output_folder parameter
  • What files are generated and their purpose
  • Example usage

This would improve maintainability and help users understand how to use the function.

Copilot uses AI. Check for mistakes.
Comment on lines +165 to +178
dir_label = ['X','Y','Z'];
for cell_index = 1:length(idx)
sw = saturations;
sg = 1 - sw;
krg_cell = squeeze(krg(cell_index,direction,:))';
krw_cell = squeeze(krw(cell_index,direction,:))';
jfunc = leverett_j(cell_index,:);
data = [sg;krg_cell;krw_cell;jfunc];
data = data(:,end:-1:1);

fprintf(file_id,'%e\t%e\t%e\t%e\n',data);
fprintf(file_id,'/ -- KRNUM%s cell %u\n',dir_label(direction),idx(cell_index));
end
end
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

This function write_tables_for_direction has significant code duplication with a similarly named function in src/export/ogs/generate_sfn.m (lines 28-59). Both functions iterate over cells, format relative permeability data, and write tables. Consider refactoring common logic into a shared utility function to improve maintainability and reduce duplication.

Suggested change
dir_label = ['X','Y','Z'];
for cell_index = 1:length(idx)
sw = saturations;
sg = 1 - sw;
krg_cell = squeeze(krg(cell_index,direction,:))';
krw_cell = squeeze(krw(cell_index,direction,:))';
jfunc = leverett_j(cell_index,:);
data = [sg;krg_cell;krw_cell;jfunc];
data = data(:,end:-1:1);
fprintf(file_id,'%e\t%e\t%e\t%e\n',data);
fprintf(file_id,'/ -- KRNUM%s cell %u\n',dir_label(direction),idx(cell_index));
end
end
dir_label = ['X','Y','Z'];
write_rel_perm_table(file_id, idx, krw, krg, saturations, leverett_j, direction, dir_label);
end
function write_rel_perm_table(file_id, idx, krw, krg, saturations, leverett_j, direction, dir_label)
for cell_index = 1:length(idx)
sw = saturations;
sg = 1 - sw;
krg_cell = squeeze(krg(cell_index,direction,:))';
krw_cell = squeeze(krw(cell_index,direction,:))';
jfunc = leverett_j(cell_index,:);
data = [sg;krg_cell;krw_cell;jfunc];
data = data(:,end:-1:1);
fprintf(file_id,'%e\t%e\t%e\t%e\n',data);
fprintf(file_id,'/ -- KRNUM%s cell %u\n',dir_label(direction),idx(cell_index));
end
end

Copilot uses AI. Check for mistakes.
mult = strata_trapped.params.cap_pressure.mult / (dyne / centi / meter);
jfunc_line = sprintf("'WATER' 0.0 %f * * XY /", mult); % TODO: generalize 'XY'
jfunc_str = {"JFUNC",jfunc_line};
jfunc_fid = fopen([output_prefix,'JFUNC.inc'],'wb','native','UTF-8');
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

No error handling is performed after fopen. If the file cannot be opened, jfunc_fid will be -1, and subsequent operations will fail silently. Consider adding error checking or using a helper function with proper error handling.

Copilot uses AI. Check for mistakes.
"INCLUDE"
"KRNUMZ.inc /"
};
regions_fid = fopen([output_prefix,'REGIONS.inc'],'wb','native','UTF-8');
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

No error handling is performed after fopen. If the file cannot be opened, regions_fid will be -1, and subsequent operations will fail silently. Consider adding error checking or using a helper function with proper error handling.

Suggested change
regions_fid = fopen([output_prefix,'REGIONS.inc'],'wb','native','UTF-8');
regions_fid = fopen([output_prefix,'REGIONS.inc'],'wb','native','UTF-8');
if regions_fid == -1
error('Failed to open file: %s', [output_prefix,'REGIONS.inc']);
end

Copilot uses AI. Check for mistakes.
params.cap_pressure.leverett_j.func(sw));
fclose(chc_file);
end

Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The new export_opm function lacks documentation. Consider adding a comment block describing:

  • Purpose of the function
  • What the sw parameter represents
  • What format/structure the returned string has
  • How it differs from export_ogs

This would improve maintainability and help users understand how to use the function.

Suggested change
% EXPORT_OPM Export saturation/relative permeability/capillary pressure data in OPM format.
%
% Parameters:
% params - Params object containing krw, krg, and cap_pressure properties.
% sw - Vector of water saturation values (fractional, between 0 and 1).
%
% Returns:
% export_str - String with tab-separated columns: sg, krg, krw, jfunc (one row per sw value).
%
% Differences from export_ogs:
% - Returns a formatted string instead of writing to a file.
% - Uses OPM format: columns are sg, krg, krw, jfunc, tab-separated.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working feature Adding a new functionality, small or large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Export to OPM Flow

1 participant