UZF, RCH, EVT, WEL and MAW improvement #263

rubencalje · 2023-10-04T20:03:04Z

This PR adds the UZF package with a notebook. Also it adds some improvements to RCH and EVT packages, like adding an aux-parameter for the concentration of recharge.

Another improvement is in the MAW and WEL packages. This should have been another PR, but it isn't. These improvements are:

Distribute k based on kh-value in wel_from_df
Add multiplier to wel_from_df, so q can be a timeseries
Add aux to maw_from_df
Improve speed by using gdf_to_grid instead of slow gwf.modelgrid.intersect
becuase it is now faster, I removed the waitbar (tqdm)
maw-cells can be added to top layer as well (was not possible before I think)

Distribute k based on kh-value in wel_from_df Add multiplier to wel_from_df, so q can be a timeseries Add aux to maw_from_df Improve speed by using gdf_to_grid instead of slow gwf.modelgrid.intersect becuase it is fast, remove tqdm maw-cells can be added to top layer as well (was not possible before)

OnnoEbbens

Good job! Just one suggestion and an unfinished comment.

OnnoEbbens · 2023-10-05T09:06:05Z

nlmod/dims/grid.py

@@ -840,6 +836,31 @@ def lcid_to_reclist(
    return reclist


+def cols_to_reclist(ds, cellids, *args, cellid_column=0):


I think at some point this function should replace the lrc_to_reclist and the lcid_to_reclist functions because they something similar. I would not do it now but I can make an issue of this if you agree?

OnnoEbbens · 2023-10-05T09:10:39Z

nlmod/gwf/wells.py

+
+
+def _get_layer_multiplier_for_well(cid, well_top, well_bot, ml_top, ml_bot, ml_kh):
+    """Get a factor for each layer that a well"""


Sentence not finished

dbrakenhoff · 2023-10-05T09:13:18Z

nlmod/gwf/gwf.py

+    logger.info("creating mf6 UZF")
+
+    # create uzf package
+    evt = recharge.ds_to_uzf(gwf, ds, pname=pname, **kwargs)


maybe call it uzf?

dbrakenhoff

Nice! Lots of improvements, I left some comments to consider, but nothing major.

dbrakenhoff · 2023-10-05T09:17:18Z

nlmod/gwf/recharge.py

+        A DataArray with integer values, set to one for land surface cells indicating
+        that boundary conditions can be applied and data can be specified in the PERIOD
+        block. A value of 0 specifies a non-land surface cell. Landflag is determined
+        from ds when it is None. The default is None.


How is this determined?

dbrakenhoff · 2023-10-05T09:17:49Z

nlmod/gwf/recharge.py

+        The potential evapotranspiration rate of the UZF cell and specified GWF cell.
+        Evapotranspiration is first removed from the unsaturated zone and any remaining
+        potential evapotranspiration is applied to the saturated zone. only
+        used if SIMULATE_ET. When passed as string, the array is obtained from ds. The


simulate_et=True

dbrakenhoff · 2023-10-05T09:18:26Z

nlmod/gwf/recharge.py

+        None.
+    extwc : float, optional
+        The evapotranspiration extinction water content of the UZF cell. Only used if
+        SIMULATE_ET and UNSAT_ETWC. Set to thtr when None. The default is None.


mention these should be set to True

Maybe nitpicking, but I think this makes it more clear these values should be set in in this function?

dbrakenhoff · 2023-10-05T09:21:16Z

nlmod/gwf/recharge.py

+        The depths at which observations of the water depth in each cell are added. The
+        user-specified depth must be greater than or equal to zero and less than the
+        thickness of GWF cellid (TOP - BOT).
+        The


Maybe the intention, but we should explain that this creates a CSV output file with water contents for each UZF cell?

obs_z is missing from the docstring

dbrakenhoff · 2023-10-05T09:21:37Z

nlmod/gwf/recharge.py

+        user-specified depth must be greater than or equal to zero and less than the
+        thickness of GWF cellid (TOP - BOT).
+        The
+    ** kwargs : dict


Are there any kwargs left :P?

dbrakenhoff · 2023-10-05T09:25:24Z

nlmod/gwf/recharge.py

+
+    # perioddata : [iuzno, finf, pet, extdp, extwc, ha, hroot, rootact, aux]
+    finf_name_arr, uzf_unique_dic = _get_unique_series(ds, finf, "finf")
+    finf = "rch_name"


Is this an internal data variable? One that is set by nlmod functions? Maybe we should consider some convention for naming these internal data variables, to make it clear this is something nlmod has created and needs for building models?

Are there any other similar data vars?

I copied the approach from RCH and EVT. However, I think we should not add these variables to ds. I will make an issue about this.

dbrakenhoff · 2023-10-05T09:32:15Z

nlmod/gwf/recharge.py

+        raise (ValueError("There cannot be nan's in the DataFrame"))
+    # set the first value for the start-time as well
+    df.loc[pd.to_datetime(ds.time.start)] = df.iloc[0]
+    # combine the values with the start of each period, and ste the last value to 0.0


typo ste = set

dbrakenhoff · 2023-10-05T09:35:27Z

nlmod/gwf/wells.py

+        ml_bot = gwf.dis.botm.array
+        kh = gwf.npf.k.array
+        layer = range(gwf.dis.nlay.array)
+    else:


maybe raise an Exception if both are None?

dbrakenhoff · 2023-10-05T09:36:27Z

nlmod/gwf/wells.py

+        # the well does not cross any of the layers. Just return an array of zeros.
+        multiplier = kd
+    else:
+        # devide by the total kd to get a factor


typo divide

dbrakenhoff · 2023-10-05T09:42:22Z

tests/test_010_wells.py

+    wells.loc[1] = 200, 150, -20, -30, -300.0
+
+    sim, gwf = get_sim_and_gwf()
+    nlmod.gwf.wells.wel_from_df(wells, gwf, auxmultname=None)


What's the difference between the auxmultname=None and setting it to "multiplier"? In both cases a multiplier is applied, but either via AUX variables or just directly in the perioddata?

Maybe a docstring in wel_from_df() would help clear that up

Yes, you are right. I added a docstring.

rubencalje added 9 commits September 27, 2023 17:28

Add first version of uzf!

1ab55d9

Add uzf observations and notebook

f2c0c6a

Improve ds_to_rch and ds_to_evt

a51eaf9

Add aux to rch adn evt and improve check for need for timeseries

056b836

solve bug where timeseries were not written

d22fdc7

Remove some metadata from notebooks

802b5ee

Fix bugs found by tests, and add extra tests

0cdbbdb

Fix codacy issues and some spelling in the uzf-notebook

d6041c1

rubencalje requested review from dbrakenhoff and OnnoEbbens October 4, 2023 21:08

OnnoEbbens approved these changes Oct 5, 2023

View reviewed changes

dbrakenhoff reviewed Oct 5, 2023

View reviewed changes

rubencalje added 2 commits October 5, 2023 20:05

Process comments and add docstrings to wells.py

330ec62

Solve codacy stuff and remove some leftover code

72c6ae7

OnnoEbbens mentioned this pull request Oct 6, 2023

check reclist functions #265

Open

rubencalje merged commit 66aa48a into dev Oct 6, 2023
3 checks passed

rubencalje deleted the uzf branch October 6, 2023 11:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UZF, RCH, EVT, WEL and MAW improvement #263

UZF, RCH, EVT, WEL and MAW improvement #263

rubencalje commented Oct 4, 2023

OnnoEbbens left a comment

OnnoEbbens Oct 5, 2023

rubencalje Oct 5, 2023

OnnoEbbens Oct 5, 2023

dbrakenhoff Oct 5, 2023

dbrakenhoff left a comment

dbrakenhoff Oct 5, 2023

dbrakenhoff Oct 5, 2023

dbrakenhoff Oct 5, 2023

dbrakenhoff Oct 5, 2023

dbrakenhoff Oct 5, 2023

dbrakenhoff Oct 5, 2023

dbrakenhoff Oct 5, 2023

dbrakenhoff Oct 5, 2023

dbrakenhoff Oct 5, 2023

rubencalje Oct 5, 2023

dbrakenhoff Oct 5, 2023

dbrakenhoff Oct 5, 2023

dbrakenhoff Oct 5, 2023

dbrakenhoff Oct 5, 2023

rubencalje Oct 5, 2023

		@@ -840,6 +836,31 @@ def lcid_to_reclist(
		return reclist


		def cols_to_reclist(ds, cellids, *args, cellid_column=0):



		def _get_layer_multiplier_for_well(cid, well_top, well_bot, ml_top, ml_bot, ml_kh):
		"""Get a factor for each layer that a well"""

UZF, RCH, EVT, WEL and MAW improvement #263

UZF, RCH, EVT, WEL and MAW improvement #263

Conversation

rubencalje commented Oct 4, 2023

OnnoEbbens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbrakenhoff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment