Skip to content

Overwrite existing nexus output NetCDF file - #956

Open
robertbartel wants to merge 4 commits into
NOAA-OWP:masterfrom
robertbartel:f/overwrite_nexus_netcdf_file/main
Open

Overwrite existing nexus output NetCDF file#956
robertbartel wants to merge 4 commits into
NOAA-OWP:masterfrom
robertbartel:f/overwrite_nexus_netcdf_file/main

Conversation

@robertbartel

Copy link
Copy Markdown
Contributor

Per feedback, and after experiencing negative side effects first hand, modifying behavior for writing nexus output to single NetCDF file to overwrite a file if one already exists, rather than bailing with an error. Notifying the user, though, if this happens.

Make sure ngen_output can see the main utilities include directory to
get access to things there like FileChecker.h.
Have PerFormulationNexusOutputMgr overwrite existing files rather than
bailing with an error, but warn user.
Add test case for PerFormulationNexusOutputMgr to make sure newly added
overwrite functionality for existing files works as expected.
@robertbartel robertbartel added the enhancement New feature or request label Apr 22, 2026
@PhilMiller

Copy link
Copy Markdown
Contributor

This would have saved me at least a few spuriously failed debugging runs, so in that sense I like it.

It's the kind of behavior that will bite real users much harder, though, because it potentially destroys the output of a long, expensive run if they make a mistake like starting a new run in the wrong directory, or copying a config file and neglecting an update.

I'm OK with having a behavior to overwrite outputs be available, but I think it needs to default off in some fashion. That could be controlled by a compile-time configuration, or something set at runtime via command line, environment variable, or in the realization config file. Environment variable is probably the easiest to add.

@robertbartel

robertbartel commented Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

This would have saved me at least a few spuriously failed debugging runs, so in that sense I like it.

It's the kind of behavior that will bite real users much harder, though, because it potentially destroys the output of a long, expensive run if they make a mistake like starting a new run in the wrong directory, or copying a config file and neglecting an update.

Thanks, @PhilMiller. I think that was largely my initial mindset, but to my surprise @ajkhattak felt differently and previously had asked me why we weren't just overwriting the file (and pointed out that with CSV output, we don't crash when things already exist). After then also having some similar spurious testing failures myself due to this, it seemed perhaps to make more sense to just clobber.

But, I wanted to use this PR to get more feedback from several people on what the right behavior for this should be. I'll tag a few more to loop them in and get their thoughts (@glitt13, @seyounger, @jduckerOWP), but feel free to add anyone I've left out.

I'm OK with having a behavior to overwrite outputs be available, but I think it needs to default off in some fashion. That could be controlled by a compile-time configuration, or something set at runtime via command line, environment variable, or in the realization config file. Environment variable is probably the easiest to add.

This seems sensible. Personally I'm not sure compile-time options make sense though ... I think the effect there could be the worst of both worlds (we're introducing the possibility of many people on both the overwrite and safeguard sides potentially being disappointed when the opposite of what they want happens). But those the other control mechanisms sound reasonable.

@ajkhattak

Copy link
Copy Markdown
Contributor

I think either approach is fine as long as the behavior is consistent for both the divides and nexus output files. Another option would be to make it part of the realization file, for example (not sure how much effort is needed for this):

output:
  dir: /path/to/output_root   # replace the current output_root

  divides:
    enabled: true             # Enable/disable writing divides outputs
    overwrite: false          # Overwrite existing divides files if they exist

  nexus:
    enabled: true             # Enable/disable writing nexus outputs
    overwrite: false          # Overwrite existing nexus files if they exist

my two cents...

@aaraney

aaraney commented Apr 27, 2026

Copy link
Copy Markdown
Member

@ajkhattak felt differently and previously had asked me why we weren't just overwriting the file (and pointed out that with CSV output, we don't crash when things already exist).

I share @ajkhattak's view that files should be overwritten by default.

I'd advocate for adding this as a cli flag (e.g. --no-overwrite-outputs).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants