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

Make a principled decision on None versus "none" in MF6 options #1024

Open
Huite opened this issue May 8, 2024 · 0 comments
Open

Make a principled decision on None versus "none" in MF6 options #1024

Huite opened this issue May 8, 2024 · 0 comments

Comments

@Huite
Copy link
Contributor

Huite commented May 8, 2024

I just noticed that in the imod.mf6.Solution class there are some ambiguities when it comes to None options.

Options like reordering_method or print_option have as acceptable MF6 values: {NONE, RCM, MD} and {NONE, SUMMARY, ALL} respectively (and we prefer lower case).

In many cases, NONE is the default option. Which means that you may omit it from the MF6 IMS file.
However, the print_option has as its default value "summary". So to silence it, you need to set to ensure PRINT OPTION NONE is written into the file, which requires providing the value "none" as a string.

To demonstrate:

        imod.mf6.Solution(
            modelnames=["GWF_1"],
            outer_dvclose=4,
            outer_maximum=500,
            inner_dvclose=1.0e-4,
            inner_rclose=0.001,
            inner_maximum=100,
            linear_acceleration="cg",
            print_option=None,
        )

To actually silence, you need:

```python
        imod.mf6.Solution(
            modelnames=["GWF_1"],
            outer_dvclose=4,
            outer_maximum=500,
            inner_dvclose=1.0e-4,
            inner_rclose=0.001,
            inner_maximum=100,
            linear_acceleration="cg",
            print_option="none",
        )

Results in no entry being written in the IMS file (due to how the jinja rendering works), which means MF6 will default to SUMMARY instead.

In other cases, allowing non-string None works fine because it results in no value being written --> mf6 uses the NONE default.
Really, the issue is that MF6 choose the word None rather than Null or whatever, and None happens to be a Python keyword. Matplotlib has something similar, where if you don't want to show faces or edges on a plot, you set facecolor="none".

I've added an OptionSchema in #1018. It currently ignores None values, since technically many of our options currently have None as a default argument. Perhaps the easiest solution is the following:

  • Do not allow None in the OptionSchema validate.
  • Update the default argument for these options where they are currently None, and replace them by "none".
  • Add "none" to the allowed values in the schema.
  • Update docstrings carefully, explicitly requiring string entries.

This will still result in some frustration as people will likely write None instead of "none". The OptionSchema validation error should probably clarify the type though since it doesn't communicate the difference nicely:

    - Invalid option: None. Valid options are: none, summary, all

Since it will lower strings otherwise, so "None" would be allowed.

Maybe:

            expected = type(next(iter(self.options)))
            raise ValidationError(
                f"Invalid option: {value} of type {type(value).__name__}. Valid options are: {valid_values} of type {expected.__name__}"
            )

This only works if not multiple types are allowed.

(The DTypeSchema won't suffice here btw since it allows None values.)

Defaulting to "none" as a string has the added benefit of a more verbose and more descriptive IMS file -- this is somewhat philosophical and maybe warrants a paragraph in the developers docs: obviously MF6 treats its input files as a user facing, and they don't want to burden the user with filling in these values. However, to us the Python API is user facing and the files are just data transfer.

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

No branches or pull requests

1 participant