pecan icon indicating copy to clipboard operation
pecan copied to clipboard

Remove assumption that $modeloutdir == $outdir / "out"

Open infotroph opened this issue 10 months ago • 2 comments

Me in code comments of https://github.com/ccmmf/workflows/pull/1 :

<!-- TODO at least one function hardcodes an assumption that model output
   lives in `file.path(settings$outdir, "out", runid)`. We should fix this,
   but meanwhile name your settings$host$outdir by adding
   "/out" to the value in settings$outdir. -->

Resulting discussion:

dlebauer on Jan 8

Here are all of the occurrences in R/R files (excluding inst, workflows, Rscripts, tests). https://github.com/search?q=repo%3APecanProject%2Fpecan+%22settings%24outdir%2C+%5C%22out%5C%22%22+path%3AR%2F.R&type=code

It looks like this assumption is built into / enforced in check.settings. If the config doesn't include a modeloutdir it will be created.

I think the fix would be to replace file.path(settings$outdir, "out"), with settings$modeloutdir in these two files.

https://github.com/PecanProject/pecan/blob/882766d7a010a091b8de30075951c57e11cb43e5/modules/uncertainty/R/run.ensemble.analysis.R#L253 https://github.com/PecanProject/pecan/blob/882766d7a010a091b8de30075951c57e11cb43e5/modules/assim.sequential/R/sda.enkf_refactored.R#L382 If that's correct, I can create an issue. How important is this fix for CCMMF?

@infotroph last month

Not important to fix for CCMMF.

replacing file.path(settings$outdir, "out"), with settings$modeloutdir seems reasonable but should include making sure the differences are documented in the XML chapter -- I'm still fuzzy on what should use outdir vs modeloutdir vs host$outdir.

@mdietze last month

Agree that file.path(settings$outdir, "out") should only be hard coded in the settings check as the default for modeloutdir if a specific value is not provided.

In terms of use of these file path variables, outdir is the main workflow folder, modeloutdir is where workflow-specific model output is stored and defaults to file.path(settings$outdir, "out"), rundir is analogous for inputs/configs and defaults to file.path(settings$outdir, "run"), and the analogous paths in the host section is where these paths would be located on a remote host.

infotroph avatar Feb 13 '25 14:02 infotroph

Wouldn't settings inherit the outdir value from the ~yml~ xml (edit) file? Am I on the right path here?

If I'm correct and that's how settings work then we can simply assign it via the parsed ~yml~ xml (edit) tree as we do with other structures like in all of the above recommended lines. Also, would it work if we replace the above lines with a specific path like :

read.output( ... , settings$modeloutdir)
# After we inherit settings from check.all.settings

Sweetdevil144 avatar Feb 13 '25 17:02 Sweetdevil144

Wouldn't settings inherit the outdir value from the yml file?

Yes, that's the intended behavior (well, XML not YML), and the bug is that these are places we're hardcoding a value that ignores any user-provided modeloutdir. Very simple fix if we can be sure modeloutdir is already set at those moments and can be used unconditioanlly, pretty simple if we also need to handle providing a default when modeloutdir is unset (in which case outdir/"out" seems like a fine default to use).

infotroph avatar Feb 13 '25 20:02 infotroph