Discrepancy in treatment of observation variable `irefsp`

[jbellino-usgs]

Hey guys, sorry to have missed the deadline for the recent release of flopy, but I wanted to point out that the variable irefsp is treated as a zero-indexed variable when writing to file in the ModflowHob package (though not explicitly so-stated in the docstrings) and as a one-indexed variable in the ModflowFlwob package. I'd be more than happy to submit a pull request, but wanted to verify that this was not intended behavior. I'd also clarify this in the docstring to explicitly state whether to the user should supply a zero-indexed value or one-indexed value, depending on which is preferred.

flopy/flopy/modflow/mfhob.py

Line 271 in e4d9397

line += '{:10d} '.format(t['irefsp'] + 1)

flopy/flopy/modflow/mfflwob.py

Lines 277 to 280 in e4d9397

	line = '{}{:10d}{:10.4g} {:10.4g}\n'.format(self.obsnam[c],
	self.irefsp[c],
	self.toffset[c],
	self.flwobs[c])

[jdhughes-dev]

I think we want zero-based for ModflowFlwob. Feel free to submit a pull request. Also ideally it would be nice to have something like the Head observation object like we have for ModflowHob. Feel free to think about how to implement a flow observation object.

[jbellino-usgs]

That sounds good Joe, I'll probably make a submission to address the zero-based index thing and then think about the Head observation object idea when I get some time. Also, is there any appetite for a load method for ModflowFlwob? I wrote one up based on the method for ModflowHob and have been using it for the Floridan model.

[jdhughes-dev]

I think we would also want a load method.

[jbellino-usgs]

The idea of a FlowObservation object was interesting to me so I wrote some code to test out what it would look like. Basically the new observation object would consist of a cell group, with one or more cells, and the associated timeseries data. All of the tricky variables (nqotfb, nqobfb, etc.) would be computed by the new ModflowFlwob object upon instantiation. Let me know how you'd like to proceed and I'll share the code so you guys can have a look and see if it makes sense to do it that way.

[jbellino-usgs]

As a matter of fact, here's a link to the main commit in my repo that implements the changes. Note that these changes were made prior to the commits we merged today that finalized the load functionality so the actual code will have to be updated to reflect those changes.