flopy icon indicating copy to clipboard operation
flopy copied to clipboard

Improve mp6 and bug fix on mt3dms

Open hansonmcoombs opened this issue 3 years ago • 11 comments

Fix a bug in mt3dms

  • mt3d-usgs text options being written despite specifying mt3dms

Fix a number of bugs in mp6:

  • allow modpath creation without modflow model
    • removing unnecessary links (e.g. to nlay) to parent model
    • reading nlay, nrow.... from dis file
  • ensure user passed variable (via kwargs) are prefered over variables from modflow model
    • head file
    • budget file
    • dis file
    • layertype
    • ibound
  • change several default kwargs to better represent use
    • e.g. ibound=1 to ibound=None, where ibound=None prefers read from passed modflow model
  • test t081_test.py

Add pymake (mfpymake) to the following pages to allow the tests to run if the user creates an environment from these files.

  • etc/environment.yml
  • etc/requirements.full.pip.txt
  • etc/requirements.pip.txt
  • etc/requirements.windows.pip.txt

implement pandas writing for modpath particles to increase speed:

  • test t082_test.py

A final note:

  • I did not run python pull_request_prepare.py before each commit (sorry first pull request here), so there is a final commit to fix this problem.

I had three failed tests these failed on the fresh fork of the development branch:

  • t039_test.py::test_zonebudget_6
  • t504_test.py::test001e_uzf_3lay
  • t504_test.py::test_mf6_output

hansonmcoombs avatar Feb 16 '22 04:02 hansonmcoombs

Codecov Report

Merging #1354 (6a5c8d6) into develop (1ac02d2) will increase coverage by 0.0%. The diff coverage is 87.9%.

@@           Coverage Diff           @@
##           develop   #1354   +/-   ##
=======================================
  Coverage     72.4%   72.4%           
=======================================
  Files          251     251           
  Lines        54146   54216   +70     
=======================================
+ Hits         39236   39300   +64     
- Misses       14910   14916    +6     
Impacted Files Coverage Δ
flopy/mt3d/mtbtn.py 95.7% <72.2%> (+<0.1%) :arrow_up:
flopy/modpath/mp6bas.py 86.5% <86.4%> (+1.1%) :arrow_up:
flopy/modpath/mp6.py 83.9% <90.0%> (+0.2%) :arrow_up:
flopy/modpath/mp6sim.py 84.1% <96.9%> (+1.9%) :arrow_up:
flopy/utils/mfreadnam.py 78.2% <0.0%> (-0.5%) :arrow_down:
flopy/utils/triangle.py 65.9% <0.0%> (-0.1%) :arrow_down:
flopy/utils/geospatial_utils.py 91.8% <0.0%> (+0.2%) :arrow_up:

codecov[bot] avatar Feb 16 '22 13:02 codecov[bot]

@langevin-usgs I should have addressed all of your comments, but let me know if I've done something else dumb :). Thanks again for your help

hansonmcoombs avatar Feb 16 '22 19:02 hansonmcoombs

Thanks for addressing these things. If you can remove the mfpymake dependency and also the changes to the dbf file, we will merge it in. If there still is good reason to include a pymake dependency, then that should be done on a separate PR.

Done, should be ready for merge

Sorry that the standard practices are not better described somewhere. If you are willing to help with that, we would gladly accept your contribution on a subsequent PR.

If I get some free time I will do that; but that's only somewhat likely.

Thanks for you contribution!

hansonmcoombs avatar Mar 04 '22 00:03 hansonmcoombs

Do you have any idea why these tests are failing? Are you able to run the mp6 jupyter notebook?

christianlangevin avatar Mar 04 '22 20:03 christianlangevin

Do you have any idea why these tests are failing? Are you able to run the mp6 jupyter notebook? these tests failed because of the way that the paths are specified, previously this wasn't an issue as Modpath6 simply ignored user passed files. Setting the input files to None, or simply excluding the kwargs (defaults are None) succeeds as then the files are inherited from the Modflow object.

I have limited experience with ipython notebooks, so I tried to fix this issue, but if that causes a problem, cell 11 should read:

mp = flopy.modpath.Modpath6( modelname="ex6", exe_name="mp6", modflowmodel=m, model_ws="data", )

Instead of: mp = flopy.modpath.Modpath6( modelname="ex6", exe_name="mp6", modflowmodel=m, model_ws="data", dis_file=m.name + ".DIS", head_file=m.name + ".hed", budget_file=m.name + ".bud", )

hansonmcoombs avatar Mar 08 '22 00:03 hansonmcoombs

@hansonmcoombs would it be possible for you to merge the latest into this PR? I am working with @langevin-usgs to get this merged.

jdhughes-dev avatar Jul 26 '22 20:07 jdhughes-dev

@jdhughes-usgs Sorry about that I thought this pull request was closed ages ago. I should have merged the latest into this PR.

Let me know if there are any other issues!

hansonmcoombs avatar Jul 27 '22 23:07 hansonmcoombs

Hey @hansonmcoombs, I rebased your changes on top of develop. Looks like they will merge fine, although there's a failing test case test_plot.py::test_pathline_plot_xc with an error reading a .hed file:

 *** ERROR OPENING FILE "EXAMPLE.hed" ON UNIT    88
       SPECIFIED FILE STATUS: OLD    
       SPECIFIED FILE FORMAT: UNFORMATTED         
       SPECIFIED FILE ACCESS: STREAM              
       SPECIFIED FILE ACTION: READ                
  -- STOP EXECUTION (SMP6MAIN1OPEN)

This then causes the expected .mppth file to not get created. I'm not familiar enough yet with your work to address it myself.

Also the autotest naming is just semantic now in case you wanted to update the test filenames

wpbonelli avatar Sep 20 '22 16:09 wpbonelli

@w-bonelli Thanks for that. I belive that you are all ready have maintainer access. If I am wrong, please let me know:

I have fixed the single test that was failing:

fix(test_plot.py):: test_pathline_plot_xc

do not specify files, instead read from modflow object. Previous version of modpath6 did not read the files but instead pulled the data from the model (irrespective of whether the files were different). Now if you want to specify the file you must pass the full path as it no longer assumes file is in the Modflow objects model_ws.

If there are issues pulling the changes. the new changes are simply deleting lines 368, 369, 370:

    dis_file=f"{ml.name}.DIS",
    head_file=f"{ml.name}.hed",
    budget_file=f"{ml.name}.bud",

let me know if you have any problems

hansonmcoombs avatar Sep 20 '22 22:09 hansonmcoombs

@hansonmcoombs thanks for the help with the test, I updated the flopy3_modpath6_example notebook to reflect the same usage pattern.

Also renamed/organized modpath-related tests, used pytest-cases to define reusable test data, and reran the linter.

wpbonelli avatar Sep 21 '22 17:09 wpbonelli

Thanks heaps let me know if there's anything else you need from me

hansonmcoombs avatar Sep 22 '22 00:09 hansonmcoombs

a few more updates:

  • follow load/switch workspace/run model convention in flopy3_modpath6_example notebook
  • move MODPATH6 and MODPATH7 tests to proper (separate) files
  • trap ImportError when checking package availability in requires_pkg pytest decorator
  • fix miniconda environment cache key

wpbonelli avatar Sep 23 '22 11:09 wpbonelli