bug: set_data() method leaves stress period data intact for periods not included in dictionary

[MickeyRush]

I am reading in a simulation of 48 stress periods, averaging stress period data down to 12, and then running the new simulation. Despite building a new stress period dictionary, reducing TDIS.NPER to 12, and then using the set_data() method, flopy still writes out stress periods 13–48. Is this the intended behavior?

Steps:

1. I build an entirely new stress period dictionary for the RCH package (with keys 0–11).
2. I update TDIS.NPER to 12.
3. I use the set_data() method, passing my new stress period dictionary to RCH.
4. I write out the simulation.

Stress period data for periods 13–48 is still written, even though keys 12–47 are not included in the dictionary. (Note that this also occurs for list type data – SFR, RIV etc.)

I would expect that we'd want set_data() to entirely overwrite the stress period data.

It's not a huge problem, except that loading in that new model takes FOREVER because NPER=12. I could fix by removing the old package and creating a new one, but this seems like overkill for something that should easily be accomplished via set_data().

[wpbonelli]

Thanks for reporting @MickeyRush, I'm not sure if this is "intended" as implemented but it's definitely not desirable. Will look into it.

[wpbonelli]

It seems like for stress period data, .set_data() has dict.update()-like semantics right now. The "blessed" way to drop keys seems to be to set them to None. But this is awkward given that you've already told the simulation that TDIS only has 12 periods, why should you have to remember the old configuration had 48 and iterate to nullify the now-invalid ones?

I'm tempted to give.set_data() replacement semantics and call it a fix, but I'm worried that will break a lot of people's code, if anyone is expecting the current behavior, e.g.

# Model with 48 stress periods,
# want to update just period 20
pkg.stress_period_data.set_data({20: ...})

After the proposed fix, this would delete periods 0-19 and 21-48.

Or, someone might be iteratively building up period data with multiple calls to .set_data().

So it is probably safest to say you have to loop the old keys and set them to None. ☹️

We will keep this situation in mind for 4.x.

[MickeyRush]

Thanks for quickly hopping on this! I will give the None approach a shot.

[jlarsen-usgs]

@wpbonelli and @MickeyRush

I would recommend adding an optional flag like reset=False (as default) to the .set_data() routine; the current behavior is intentional and allows users to change boundary conditions for individual stress periods without having to remake boundary conditions for the remainder of the simulation.

I would caution against changing the default behavior. I think it'd break a decent amount of user code and If I remember right, the __setitem__ function currently uses the .set_data() routine, so changes to the default behavior might break that too.

[MickeyRush]

Good stuff, thanks guys. Adding None dictionary entries, e.g., {12: None} worked fine for now. Still end up with empty stress period blocks, but the simulation loads quickly, which is ultimately what I'm after.