feat: Add env var LUDWIG_SCHEMA_VALIDATION_POLICY to change marshmallow validation strictness

[tgaddair]

Closes #3218.

[ksbrar]

My only question is how extensively have you tested this locally? (Last thing I was fiddling around with pytest env variables and was about to try Ludwig CLI).

In particular, I want to check if there are any breakages when RAISE is set for schemas that explicitly allow additional params via JSON. That means (based on a ctrl-f):

• input features (top-level)
• output features (top-level)
• preprocessing
• augmentation

Really, just one of these cases suffices as a sanity check for the rest.

(Also, I believe we're covered but we have to be careful that any internal parameters are in fact declared on their respective schemas (e.g. proc_column for input features) - otherwise RAISE will reject them.)

[tgaddair]

It's a good point @ksbrar, I haven't tested this extensively! If you have some cycles to test it out, please take a look and let me know what you find.

[github-actions]

Unit Test Results

         6 files           6 suites   6h 45m 55s ⏱️
  4 087 tests   4 044 ✔️   43 💤 0 ❌
12 282 runs  12 147 ✔️ 135 💤 0 ❌

Results for commit 8530a26.

♻️ This comment has been updated with latest results.

[ksbrar]

I've tested out this PR. It doesn't work as is because the deprecation warning PR I merged a little while ago is actually performing the wrong task and is too aggressive in filtering parameters.

This code (#3118) will filter out and log any and every unknown parameter as deprecated - which is the incorrect thing to do, as there is a clear distinction between a purely "unknown" or invalid parameter and a "deprecated" one - before marshmallow even has a chance to raise an error about the unknown parameters.

I just pushed a simple fix that gets us to what we want (a user sets this environment flag to raise, and they will see marshmallow validation errors for unknown parameters), but I want to sync with @justinxzhao (or @tgaddair ) about how deprecated parameters should be marked and parsed.

[justinxzhao]

Synced offline -- upgrading the config, removing known deprecated parameters, and logging warnings for them is handled by upgrade_config_dict_to_latest_version.

We should be good to proceed with this PR (just blocked on writing a test).

Follow-up PRs to continue progress towards making STRICT the default policy for Ludwig configs:

1. Make STRICT the default
   • Clean up preload deprecation logging (feat: Raise deprecation warnings for unknown parameters #3118) as it's no longer needed
   • Set policy to STRICT by default
   • set additionalProperties on all jsonschema to False
   • Remove os.environ var
2. Remove explicit call to JSON validation and rely purely on Marshmallow deserialziation
   • Remove call to check_schema() in model_types::base.py (assuming parity)
   • Keep check_schema() around for unit tests to verify pure-JSON-validation behavior.

[ksbrar]

@justinxzhao I spent some time and just couldn't get the environment variable switching to work properly for the test. If someone else wants to give it a go, here is some rough code showing a couple different attempts. I can clean it up/give some clarification if need be:

def rreload(module):
    from importlib import reload

    """Recursively reload modules."""
    reload(module)
    for attribute_name in dir(module):
        attribute = getattr(module, attribute_name)
        if type(attribute) is ModuleType:
            rreload(attribute)


# @mock.patch.dict(os.environ, {LUDWIG_SCHEMA_UNKNOWN_POLICY: "raise"})
def test_user_config_validation_policy():
    from importlib import reload, import_module
    import sys
    from ludwig.constants import LUDWIG_SCHEMA_VALIDATION_POLICY

    # from ludwig.schema.model_types import base

    config = {
        "input_features": [{"name": "foo", "type": "category"}],
        "output_features": [category_feature(encoder={"vocab_size": 2}, reduce_input="sum")],
        "combiner": {"type": "concat", "output_size": 14},
        "trainer": {"unknown": "parameter"},
    }

    # Test default policy (unknown parameters are ignored):
    # ModelConfig.from_dict(config)

    # reload(lsb)

    print(os.environ.get(LUDWIG_SCHEMA_VALIDATION_POLICY))
    os.environ[LUDWIG_SCHEMA_VALIDATION_POLICY] = RAISE
    print(os.environ.get(LUDWIG_SCHEMA_VALIDATION_POLICY))
    # reload(sys.modules["ludwig.schema.model_types.base"])
    # reload(sys.modules["ludwig.schema.model_types.ecd"])
    # reload(sys.modules["ludwig.schema.utils"])
    # reload(sys.modules["ludwig.schema"])

    # mod = import_module("ludwig.schema.model_types.base")
    import ludwig.schema.model_types.base as base

    rreload(base)

    # for name, module in sys.modules.items():
    #     if "ludwig.schema" in name:
    #         reload(module)

    # Test RAISE policy:
    # monkeypatch.setenv(LUDWIG_SCHEMA_UNKNOWN_POLICY, "raise")
    with pytest.raises(ConfigValidationError):
        base.ModelConfig.from_dict(config)

But changing the validation policy is easy enough to verify locally in a terminal, so I think we can proceed with just landing this PR.

[ksbrar]

Good to go from my standpoint.