Modflow-2005 style model (and mf6) instances can't be copied

[aleaf]

Copying (or deep copying) a model instance results in an error within copy.py:

import copy
import flopy

m = flopy.modflow.Modflow()
cp = copy.copy(m)
TypeError: 'NoneType' object is not callable

I think this is because of how BaseModel.__getattr__() is implemented. Unhandled attributes are returned as None, which causes the error in copy.py. Doing some googling and looking at the Learning Python book, I think __getattr__ is supposed to raise an AttributeError for unhandled attributes, as it is the last stop in the attribute lookup sequence. The problem with doing this right now is there are at least a few (maybe many) instances where undefined attributes are called. For example, ModflowDis.__init__ calls model._rotation, which may not be defined. Currently, when loading a model, this call gets passed thru __getattr__, which returns None, but would otherwise result in an AttributeError.

It seems like we shouldn't be relying on __getattr__ to initialize any undefined attributes that are called, and should instead initialize them to None in the model.__init__ methods. But maybe there's a better way.

I think we do want to be able to copy model instances. In addition to any user cases, this could be important in refactoring the test suite using fixtures to be more efficient.

It looks like this issue also applies to mf6 simulation, model and package instances. Modflow-2005 style Package instances copy fine, presumably because they don't overload __getattr__.

[christianlangevin]

@aleaf-usgs, thanks for looking into this. I can see that it may not be ideal to just let __getattr__() return a None for anything and everything that is asked for but doesn't exist. But if in the model constructor, we were to initialize self.rotation = None, would copy.py be able to handle that? Asking for model.rotation would return a None in both cases. I like the idea of defining all model attributes, even if it's just to None.

[aleaf]

I think the problem is that copy.py expects an AttributeError when looking for attributes that don't exist. The actual copy.py error has to do __reduce_ex__, not anything in flopy. When it gets None instead, it breaks. When I make these changes, the above code works, as does:

m2 = flopy.modflow.Modflow.load('freyberg.nam',
                        model_ws='../examples/data/freyberg_multilayer_transient/')
dcp = copy.deepcopy(m2)

But then the tests reveal other instances where non-existent attributes, especially packages, are called. For example:

======================================================================
ERROR: t004_test_utilarray.test_append_mflist
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/aleaf/anaconda3/envs/gis/lib/python3.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/Users/aleaf/Documents/GitHub/flopy/autotest/t004_test_utilarray.py", line 679, in test_append_mflist
    ml.write_input()
  File "/Users/aleaf/Documents/GitHub/flopy/flopy/mbase.py", line 1249, in write_input
    p.write_file()
  File "/Users/aleaf/Documents/GitHub/flopy/flopy/modflow/mfwel.py", line 304, in write_file
    self.stress_period_data.write_transient(f_wel)
  File "/Users/aleaf/Documents/GitHub/flopy/flopy/utils/util_list.py", line 701, in write_transient
    self.__tofile(f, kper_data)
  File "/Users/aleaf/Documents/GitHub/flopy/flopy/utils/util_list.py", line 731, in __tofile
    np.savetxt(f, d, fmt=self.fmt_string, delimiter='')
  File "/Users/aleaf/Documents/GitHub/flopy/flopy/utils/util_list.py", line 254, in fmt_string
    if self.package.parent.bas6 is not None:
  File "/Users/aleaf/Documents/GitHub/flopy/flopy/mbase.py", line 512, in __getattr__
    raise AttributeError(item)
AttributeError: bas6

To make the tests work, these instances would have to be changed to self.has_package(), or the attributes initialized in the constructors. I'm not sure how to fix the problem in mf6.

[christianlangevin]

I understand now. A deep copy is going to be tricky when we do all this low-level mucking around with attributes. I see that you've made those changes to make a deep copy of m2. With that deep copy, does m2.bas6 exist? Can you tell if it is identical to m2? Or do we have a bunch of problems under the hood? I guess that would be a good test. Does an m2.run_model() give the same results as a dcp.run_model()?

[aleaf]

the heads from running the deep copy match those of running the original. Otherwise, comparing the two objects in memory isn't super straightforward.

import os
import copy
import numpy as np
import flopy

m = flopy.modflow.Modflow()
cp = copy.copy(m)
dcp = copy.deepcopy(m)

for dir in ['m2', 'dcp']:
    if not os.path.isdir(dir):
        os.makedirs(dir)
m2 = flopy.modflow.Modflow.load('freyberg.nam',
                        model_ws='../examples/data/freyberg_multilayer_transient/')
m2.exe_name = 'mfnwt'
m2.model_ws = 'm2'
m2.write_input()
m2.run_model()

dcp = copy.deepcopy(m2)
dcp.model_ws = 'dcp'
dcp.write_input()
dcp.run_model()

from flopy.utils import binaryfile as bf
hdsobj_m2 = bf.HeadFile('m2/freyberg.hds')
kstpkper = hdsobj_m2.get_kstpkper()

hdsobj_dcp = bf.HeadFile('dcp/freyberg.hds')

for kstp, kper in kstpkper:
    hds_m2 = hdsobj_m2.get_data(kstpkper=(kstp, kper))
    hds_dcp = hdsobj_dcp.get_data(kstpkper=(kstp, kper))
    assert np.allclose(hds_m2, hds_dcp)

[christianlangevin]

Well that's encouraging, and I do like the idea of supporting copying. So maybe with these few changes that you've made, and with some additional changes, we can more generally support copying. At first I thought that maybe we should just refactor and use the has_package() method to check to see if the package is attached to the model. But then I remembered that the models have a mfnam_packages dictionary that we could use to set and initialize package attributes to None for all the packages that we support. Would do think of that? Or do you think refactoring to use has_package() would be a better route?

[aleaf]

Thanks for the mfnam_packages suggestion. With these changes, the test suite passes and Modflow instances can be copied. I'm not sure how to implement this on mf6 objects though. I tried a few things but ran into recursion errors. @spaulins-usgs, any ideas on how we might implement this? Basically, we need __getattr__ to raise an AttributeError for undefined attributes, except for supported packages.

[spaulins-usgs]

@aleaf sorry I somehow missed this question. It looks like the recursion errors are due to mf6 getattr code assuming that init has already been called. Copy does not call init (it calls new and then builds the object attributes itself). I was able to make copy work by adding an initialization check to the getattr code. For the simulation getattr I did this:

if item == 'structure' or not hasattr(self, 'structure'):
raise AttributeError(item)

This checks for the existence of the attribute 'structure', which should always exist if init was called, and raises an AttributeError if it has not. This avoids the infinite recursion of trying to access an uninitialized 'structure' attribute from within getattr.

So far I have fixed the simulation and model object, though only copy works so far (deepcopy is still failing). When I have a chance I will try to get copy working for the package and data objects as well as supporting classes. Hopefully that will fix deepcopy as well.

[aleaf]

@spaulins-usgs cool, thanks for looking into that. Like I said, I have a fix for the mf-2005 side that I can push when you get the mf6 side figured out.

[spaulins-usgs]

@aleaf I got copy working on the mf6 side, but deepcopy still has some issues. Both copy and deepcopy run without reporting errors, but deepcopy in certain cases does not correctly copy subpackages.

For example, I created a well package with a time series subpackage. If I deepcopy the well package, it correctly copies the well package and the time series subpackage (which the well package has a reference to). However, if I deepcopy the model object, it forks the timeseries subpackage. The copied model object has a reference to one fork which is missing most of it properties. The copied package object has a reference to the other fork which contains the full timeseries subpackage.

I don't know why deepcopy is doing this, though I suspect it is getting confused because references to the timeseries subpackage are being held in multiple places. I am currently trying to think of a reasonable solution that might solve this without loss of functionality. The modified flopy code with copy working and deepcopy being buggy can be found here:

https://github.com/spaulins-usgs/flopy

[jdhughes-dev]

@spaulins-usgs can we close this issue?