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

json config files not using strings get hard to understand execption #74

Closed
behrica opened this issue Jan 26, 2025 · 4 comments · Fixed by #75
Closed

json config files not using strings get hard to understand execption #74

behrica opened this issue Jan 26, 2025 · 4 comments · Fixed by #75

Comments

@behrica
Copy link

behrica commented Jan 26, 2025

a config file like:

{
    "config": {
        "components": [
            {
                "name": "trainer",
                "type": "org.tribuo.classification.dtree.CARTClassificationTrainer",
                "properties": {
                    "maxDepth": 6,
                    "seed": "12345"
                }
            }
        ]
    }
}

throws an exception like:

Cannot invoke "String.hashCode()" because "this.value" is null

because maxDepth is not given as a String, but as a number.
which is not very helpfull for a user to find an error in this config file

@Craigacp
Copy link
Member

We can fix the exception to be more useful. It's unfortunate that we can't use the types in things like json and edn very well, OLCUT is extremely lazy so we avoid anything that looks like type checking until the user wants to instantiate an object, and I worry that if we loaded things in with types then converted to strings and then back out to numerical types we might not get exactly the same things (e.g. if we went through float when the target type was double).

@Craigacp
Copy link
Member

Something like this?

com.oracle.labs.mlrg.olcut.config.io.ConfigLoaderException: Invalid value in component a, propertylist doubleArray, node = [{"item":1.0E-16},{"item":2.0E-16},{"item":"3.16"}], all OLCUT values must be strings, numerical types are not parsed.

@Craigacp
Copy link
Member

Huh, I'd added a sensible exception for property maps when I added JSON support, but not for property lists or properties. That's a straightforward fix.

@Craigacp Craigacp changed the title json config files not using strings get ahrd to understand execption json config files not using strings get hard to understand execption Jan 26, 2025
@enragedginger
Copy link

Thank you!

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 a pull request may close this issue.

3 participants