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

refactor(createpackages): use jinja for mf6 module code generation #2333

Open
wants to merge 73 commits into
base: develop
Choose a base branch
from

Conversation

wpbonelli
Copy link
Member

@wpbonelli wpbonelli commented Oct 10, 2024

Carved out of #2317 — just move to Jinja as an initial step, type hints can come afterwards as they will take some effort to get right. The aim here is to match the old createpackages.py capabilities without relying on mfstructure.py. This is an initial step to move mfstructure.py to a leaner representation of the input spec. mfstructure.py is still used at runtime which will need to be unraveled in followup work.

Introduce a flopy.mf6.utils.codegen module with some new machinery for loading an input specification from DFN files into an intermediate representation, and generating source code with Jinja. This is fairly general, handling quirks of the existing framework in a "shim" of Jinja filters that transform the template context before rendering. These can be removed as we rework things. The templates should also get simpler over time.

For now the load routine uses a data structure from the boltons library to represent a DFN as an unstructured (flat) map of variables before structural parsing, where variables can have duplicate names. If we could guarantee all variable names were unique by changing a few DFNs this wouldn't be needed. In any case it will not be necessary once we have structured TOML definition files.

Jinja2, boltons, tomlkit and modflow-devtools are added to a new optional dependency group codegen. These are required to use the code generation utilities — this may require CI tweaks in related repositories and should be noted by developers too.

Tentative next steps after this PR:

  1. Refactor input spec introspection

    • store the new structured DFN representation on component classes instead of the original unstructured DFN
    • once this is in place, we can simplify mfstructure.py to inspect this instead of parsing lists of strings
  2. Start using structured DFN files. We have discussed prototyping this for the time being in a way that is not visible to the user, and refining the format until ready to adopt upstream in MF6. A first draft TOML conversion script has been added — we could later convert DFNs -> TOML at code generation time, then exercise the TOML load.

  3. Explicit versioning for the DFN specification language. TOML could be considered v2 where V1 is the original DFN format.

  4. Add type hints as per feature: Adding Python type hints for input specifications #2298

  5. Drop *TemplateGenerator mechanism if possible

Miscellaneous:

@wpbonelli wpbonelli added modflow 6 dependencies Pull requests that update a dependency file maintenance labels Oct 10, 2024
@wpbonelli wpbonelli added this to the 3.9 milestone Oct 10, 2024
Copy link
Contributor

@deltamarnix deltamarnix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked through everything, but I left you a PR on your own branch with indications on how I would probably approach this. Let's take a look at it together and see if we can get rid of the shim :)

autotest/test_codegen.py Outdated Show resolved Hide resolved
docs/mf6_dev_guide.md Outdated Show resolved Hide resolved
docs/mf6_dev_guide.md Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
flopy/mf6/data/dfn/utl-tas.dfn Show resolved Hide resolved
flopy/mf6/utils/codegen/templates/context.py.jinja Outdated Show resolved Hide resolved
flopy/mf6/utils/codegen/shim.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 90.74675% with 57 lines in your changes missing coverage. Please review.

Project coverage is 74.7%. Comparing base (dc394cd) to head (32ff250).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
flopy/mf6/utils/codegen/dfn2toml.py 0.0% 30 Missing ⚠️
flopy/mf6/utils/codegen/filters.py 93.5% 15 Missing ⚠️
flopy/mf6/utils/codegen/dfn.py 96.0% 10 Missing ⚠️
flopy/mf6/utils/createpackages.py 75.0% 1 Missing ⚠️
flopy/mf6/utils/generate_classes.py 83.3% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           develop   #2333     +/-   ##
=========================================
+ Coverage     74.5%   74.7%   +0.1%     
=========================================
  Files          292     297      +5     
  Lines        59624   60654   +1030     
=========================================
+ Hits         44463   45350    +887     
- Misses       15161   15304    +143     
Files with missing lines Coverage Δ
flopy/mf6/data/mfdatastorage.py 73.4% <ø> (ø)
flopy/mf6/utils/codegen/__init__.py 100.0% <100.0%> (ø)
flopy/mf6/utils/codegen/context.py 100.0% <100.0%> (ø)
flopy/mf6/utils/createpackages.py 80.0% <75.0%> (+73.5%) ⬆️
flopy/mf6/utils/generate_classes.py 20.3% <83.3%> (+2.6%) ⬆️
flopy/mf6/utils/codegen/dfn.py 96.0% <96.0%> (ø)
flopy/mf6/utils/codegen/filters.py 93.5% <93.5%> (ø)
flopy/mf6/utils/codegen/dfn2toml.py 0.0% <0.0%> (ø)

... and 123 files with indirect coverage changes

Copy link
Contributor

@deltamarnix deltamarnix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a suggestion for removing indentation out of the macro

flopy/mf6/utils/codegen/templates/package.py.jinja Outdated Show resolved Hide resolved
@wpbonelli
Copy link
Member Author

@jdhughes-usgs @langevin-usgs I think this is about ready, but maybe it should wait til after the class if the generate classes script will be used there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file maintenance modflow 6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants