bug: CLN package list output has no separator between fields

[jdhughes-dev]

The CLN list arrays are written with np.savetxt(..., delimiter="") and a format string built by fmt_string, which joins the field formats with no separator. Fields are held apart only by the padding in each format, so a value that fills its width runs into the next value and the record can no longer be read with URWORD.

Affects the four np.savetxt calls in MfUsgCln.write_file, flopy/mfusg/mfusgcln.py:547-563 (node_prop, cln_gwc, cln_circ, cln_rect).

To reproduce

import sys
import numpy as np
from flopy.mfusg.cln_dtypes import MfUsgClnDtypes
from flopy.mfusg.mfusg import fmt_string

node_prop = np.zeros(1, dtype=MfUsgClnDtypes.get_clnnode_dtype())
node_prop[0]["ifno"] = 1234567890
node_prop[0]["iftyp"] = 1234567890
np.savetxt(sys.stdout, node_prop, fmt=fmt_string(node_prop, False), delimiter="")

12345678901234567890         0  0.00e+00 ...

The first two fields merge into one token, so the record has 13 tokens where the dtype has 14 fields.

Expected behavior

Every field is written as a separate token.

Additional context

Only the integer fields can fill their width. %10d is filled by a value of 1e9 or greater, or -1e8 or less. The float fields cannot fill theirs, because a float32 never produces more than 9 characters under %10.2e or 15 under %16.9G. A CLN model would therefore need on the order of 1e9 nodes to hit this, so it is a robustness problem rather than one that shows up in practice.

The GNC package writer had the same defect. It was fixed in #2787 by adding a sep argument to flopy.mfusg.mfusg.fmt_string, which defaults to the existing behavior, and writing the list with sep=" ". The same change applies here once that is merged.

[jdhughes-dev]

The missing separator looks like a regression from when this function was forked, rather than something that was never considered.

MfList.fmt_string in flopy/utils/util_list.py:368-371 ties the separator to the format:

if use_free:
    fmt_string = " " + " ".join(fmts)
else:
    fmt_string = "".join(fmts)

flopy.mfusg.mfusg.fmt_string is a copy of that function — same dtype dispatch, same error text — but it always concatenates and ignores its free argument when joining. So a CLN model written with free format is missing the separator that every other list package gets.

That makes the defect reachable in a way the node number threshold suggested it was not. MfUsgCln.write_file passes free=self.parent.free_format_input, so any free format CLN model is already writing output that only holds together because of the field widths.

The fix is to restore the conditional join rather than to add a separator:

if sep is None:
    sep = " " if free else ""
return sep.join(fmts)

I tried this locally and test_usg.py, test_usg_transport.py, test_gnc.py, and test_gridgen.py all pass, but that is weak evidence: mfusgcln.py is at 55% coverage and most of the tests that exercise the CLN writer skip without mfusg_gsi, so the change needs verification against a real CLN model before it goes in. Leaving it out of #2787 for that reason.

[jdhughes-dev]

Fixed in #2789 by restoring the conditional join rather than by adding a separator, so the separator follows the format of the list the way MfList.fmt_string already does it.

Verified against real CLN models. mfusg_gsi is available through get-modflow, so autotest/test_usg_transport.py runs rather than skips, and the CLN model tests pass with the change. A free format CLN node record now writes 14 tokens for its 14 fields where it previously wrote 13, and a fixed format record is unchanged because it is read by position.