Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(simulation packages): *** Breaks interface *** #1394

Merged
merged 10 commits into from
Apr 19, 2022
Merged

fix(simulation packages): *** Breaks interface *** #1394

merged 10 commits into from
Apr 19, 2022

Conversation

spaulins-usgs
Copy link
Contributor

*** Breaks interface ***

Adding different types of mvr packages will no longer require modifying flopy code. Flopy code updated to better support generic child packages. The mvr and gnc packages are now sub-packages like the obs/ts/tas packages and uses the sub-package interface
(this breaks the existing interface and may require minor modifications to end-user code). Additionally, mvt, ats, tvk, and tvs all are now sub-packages and use the sub-package interface. This could potentially break end-user code as well, though these packages are so new the impact will likely be minimal. Documentation added for adding new packages and models to flopy. Tutorial added for demonstrating a simulation with multiple models.

*** Breaks interface ***

@spaulins-usgs
Copy link
Contributor Author

How to upgrade your code to work with the new flopy interface

This Pull Request breaks the interface for creating the following packages:

ATS
GNC - if referenced in exchange package (GNC across models)
MVR - if referenced in exchange package (MVR across models)
MVT - if referenced in exchange package (MVT across models)
TVK
TVS

To fix your python code you will need to do the following:

  1. Remove the <package>_filerecord parameter from the parent package
  2. When initializing one of the sub-packages above, the first parameter is now the parent package object (instead of the model object)

Example code fixes for each package type:

ATS Package

• Remove ats_filerecord from tdis constructor
• Pass in tdis object instead of sim object to ats constructor

Previous code

tdis = flopy.mf6.ModflowTdis(sim, ats_filerecord=ats_fr, …)
ats = flopy.mf6.ModflowUtlats(sim, filename=ats_fr, …)

New code

tdis = flopy.mf6.ModflowTdis(sim, …)
ats = flopy.mf6.ModflowUtlats(tdis, filename=ats_fr, …)

GNC Package

• This PR only breaks across model “simulation level” GNC packages. Model level GNC packages do not have a parent package and therefore function the same as before.
• Remove gnc_filerecord from exchange package
• Pass in exg object instead of sim object to gnc constructor

Previous code

exg = ModflowGwfgwf(sim, gnc_filerecord=gnc_path, …)
gnc_package = ModflowGwfgnc(sim, filename=gnc_path, …)

New code

exg = ModflowGwfgwf(sim, …)
gnc_package = ModflowGwfgnc(exg, filename=gnc_path, …)

MVR Package

• This PR only breaks across model “simulation level” MVR packages. Model level MVR packages do not have a parent package and therefore function the same as before.
• Remove mvr_filerecord from exchange package
• Pass in exg object instead of sim object to mvr constructor

Previous code

exg = ModflowGwfgwf (sim, mvr_filerecord=mvr_path, …)
mvr_package = ModflowGwfmvr(sim, filename=mvr_path, …)

New code

exg = ModflowGwfgwf(sim, …)
mvr _package = ModflowGwfmvr(exg, filename=mvr_path, …)

MVT Package

• This PR only breaks across model “simulation level” MVT packages. Model level MVT packages do not have a parent package and therefore function the same as before.
• Remove mvt_filerecord from exchange package
• Pass in exg object instead of sim object to mvt constructor

Previous code

exg = ModflowGwtgwt (sim, mvt_filerecord=mvt_path, …)
mvt_package = ModflowGwtmvt(sim, filename=mvt_path, …)

New code

exg = ModflowGwtgwt(sim, …)
mvt _package = ModflowGwtmvt(exg, filename=mvt_path, …)

TVK Package

• Remove TVK_filerecord from npf constructor
• Pass in npf object instead of model object to tvk constructor

Previous code

npf = flopy.mf6.Modflownpf(model, tvk_filerecord=tvk_fr, …)
tvk = flopy.mf6.ModflowUtltvk(model, filename=tvk_fr, …)

New code

npf = flopy.mf6.Modflownpf(model, …)
tvk = flopy.mf6.ModflowUtltvk (npf, filename=tvk_fr, …)

TVS Package

• Remove tvs_filerecord from sto constructor
• Pass in sto object instead of model object to tvs constructor

Previous code

sto = flopy.mf6.Modflowsto (model, tvs_filerecord=tvs_fr, …)
tvs = flopy.mf6.ModflowUtltvs(model, filename=tvs_fr, …)

New code

sto = flopy.mf6.Modflowsto(model, …)
tvs = flopy.mf6.ModflowUtltvs(sto, filename=tvs_fr, …)

@codecov
Copy link

codecov bot commented Apr 18, 2022

Codecov Report

Merging #1394 (6288862) into develop (140d3e5) will decrease coverage by 44.762%.
The diff coverage is 45.294%.

❗ Current head 6288862 differs from pull request most recent head 1f60d31. Consider uploading reports for the commit 1f60d31 to get more accurate results

@@              Coverage Diff               @@
##           develop     #1394        +/-   ##
==============================================
- Coverage   74.810%   30.047%   -44.763%     
==============================================
  Files          249       249                
  Lines        53573     52077      -1496     
==============================================
- Hits         40078     15648     -24430     
- Misses       13495     36429     +22934     
Impacted Files Coverage Δ
flopy/mf6/data/mfdatastorage.py 64.839% <ø> (-14.081%) ⬇️
flopy/mf6/mfbase.py 60.109% <0.000%> (-25.957%) ⬇️
flopy/mf6/modflow/mfgnc.py 52.000% <ø> (+9.142%) ⬆️
flopy/mf6/modflow/mfgwf.py 100.000% <ø> (+26.923%) ⬆️
flopy/mf6/modflow/mfgwfapi.py 100.000% <ø> (+30.303%) ⬆️
flopy/mf6/modflow/mfgwfbuy.py 100.000% <ø> (+25.806%) ⬆️
flopy/mf6/modflow/mfgwfchd.py 100.000% <ø> (+30.434%) ⬆️
flopy/mf6/modflow/mfgwfcsub.py 100.000% <ø> (+32.773%) ⬆️
flopy/mf6/modflow/mfgwfdis.py 100.000% <ø> (+33.333%) ⬆️
flopy/mf6/modflow/mfgwfdisu.py 100.000% <ø> (+31.506%) ⬆️
... and 280 more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants