alex-s-gardner / GEMB

A 1D column that simulates snow/firn/ice processes and surface-atmosphere mass and energy exchanges
Apache License 2.0
5 stars 1 forks source link

Make output filename an optional input to the GEMB function #13

Open chadagreene opened 1 month ago

chadagreene commented 1 month ago

Currently, the only way to define the GEMB output data filename is by specifying something like:

S.runPfx = 'S2A1D2';

before calling

GEMB(P0, Ta0, V0, dateN, dlw0, dsw0, eAir0, pAir0, S, isrestart)

This approach buries the filename within S, the runPfx handle isn't very intuitive if you're looking for something like S.outputFilename, and it's hard to find or know how to change the filename (it's currently placed 150 lines away from the GEMB call in MASTER_RUN.m).

There's an additional layer of confusion because in the example above, S2A1D2 is not the complete name of the output file, because S2A1D2 is later automagically converted to S2A1D2_000001 inside a function called combineStrucData_GEMB.m, and then that's used by the GEMB function to create S2A1D2_000001.mat.

The GEMB function also overwrites any files that might have the same automagically created name, without warning.

Because GEMB doesn't output anything to the workspace, as a user I get this feeling that I'm giving GEMB a strangely cobbled together collection of inputs, the function does something for five or ten minutes, and then if all goes well, I might end up with a .mat file somewhere on my computer, by a name I didn't explicitly ask for, and no actual data in the workspace.

I propose changing the GEMB call to look something like this:

outputs = GEMB(P0, Ta0, V0, dateN, dlw0, dsw0, eAir0, pAir0, S, isrestart, outputFile="myfile.nc")

The idea is that

  1. GEMB should directly provide the outputs to the workspace if the user wants them, which would provide a more direct conceptual link from function input to output, and
  2. only save the data to a file if the user explicitly asks for it (and include a guardrail to protect against overwriting).
alex-s-gardner commented 1 month ago

Agreed.

We could include a flag for "force_save"

alex-s-gardner commented 1 month ago

I would have date as the first input.. hinting that the following variables are time-evolving