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

Interpolations that resolve to missing value ??? don't get passed to resolvers #1130

Open
MatteoVoges opened this issue Oct 11, 2023 · 3 comments

Comments

@MatteoVoges
Copy link
Contributor

Is your feature request related to a problem? Please describe.
I have some compare-resolver that checks if some values are equal or not. When I tried to compare against the MISSING_VALUE ???, the resolver does not get called.

Here's a simpler repro-example with a resolver, that just prints its input:

from omegaconf import OmegaConf

OmegaConf.register_new_resolver("print", print) # the default print function

config = OmegaConf.create({"a": "${print:${b}}", "b": "???"})

OmegaConf.resolve(config)
# prints nothing, but should print "???"

print(config)
# >>> {'a': '???', 'b': '???'}

I think it's a general problem with the missing value. The value for a at the end should be None and not ???. I can follow the reasons why it's implemented in that way, that if some sub-interpolations are ???, the whole interpolation gets evaluated as ???.

Describe the solution you'd like
Ideally such an interpolation should be treated as a normal value like the string ??? unless you call some function with throw_on_missing = true.

Thanks in advance for your help!

@odelalleau
Copy link
Collaborator

This is by design and unlikely to change. Allowing missing values to be passed as input to resolvers would force all resolvers to implement some handling of missing values, which we definitely don't want. What might be possible is adding a flag to register_new_resolver() to indicate that it's ok for the resolver to take a missing value as input, but I haven't looked at this code recently so I'm not sure how easy it'd be to add.

By the way this makes me realize there is an issue: I was expecting OmegaConf.resolve(config) to crash, but it didn't. This is annoying because after resolution OmegaConf.is_missing(config, "a") is True, while it is False before => this is definitely a discrepancy that should be looked into.

@MatteoVoges
Copy link
Contributor Author

What might be possible is adding a flag to register_new_resolver() to indicate that it's ok for the resolver to take a missing value as input

Is this functionality actually located on the resolver side or in the resolve logic? If so, is there like a specific if-clause that breaks for the missing value? Unfortunately I don't have time at the moment to look deeper into it...

@odelalleau
Copy link
Collaborator

Is this functionality actually located on the resolver side or in the resolve logic? If so, is there like a specific if-clause that breaks for the missing value? Unfortunately I don't have time at the moment to look deeper into it...

The error is raised during resolution, at

raise InterpolationToMissingValueError(

Currently it's not caught so it just makes the program crash. If we wanted to allow a resolver to process a missing value, I think we'd need to catch this exception at
for val, txt in self.visitSequence(maybe_seq):
and forward the information about the missing value to the resolver callback.

Note that looking at the code I noticed there's a workaround: use ${oc.select:node, default_if_missing} instead of ${node} => you can pick a default value that your resolver recognizes as indicating a missing value.

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

No branches or pull requests

2 participants