You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add optional allow_single_version Boolean arg to actx.compile #227
I don't think I understand this. Could you please elaborate? Does this have to do with sub-expressions having the same value but different metadata?
Thanks!
Sorry, I didn't spend enough time explaining. Right now, we have reasonably elaborate dispatch logic between different versions of a compiled function, based on shapes, which arguments are supplied, etc. This flag would assert that only a single version should ever exist, and calling the compiled function a second time with mismatched arguments would be an error. Does that make sense?
Question: Would the allow_single_version=True case check if the arguments are the same as when it was compiled? Or is it more of an "at your own risk" thing?
Question: Would the allow_single_version=True case check if the arguments are the same as when it was compiled? Or is it more of an "at your own risk" thing?
It should be an error at this point if len(self.program_cache) > 1.
Question: Would the allow_single_version=True case check if the arguments are the same as when it was compiled? Or is it more of an "at your own risk" thing?
It should be an error at this point if len(self.program_cache) > 1.
Does it still incur the "dispatch cost to select which implementation to use" then?
In any case, I can see the value in avoiding multiple compiles. On the user side, though, I'm not sure if it's always obvious enough when it's happening that one would know to intervene. Kind of makes me wonder if the single_version_only=True behavior should be the default? i.e., emit an error if it tries to compile multiple times, unless the caller explicitly says it's OK. Not sure how much stuff this would break, though.
Activity
kaushikcfd commentedon Apr 26, 2023
I don't think I understand this. Could you please elaborate? Does this have to do with sub-expressions having the same value but different metadata?
Thanks!
inducer commentedon Apr 26, 2023
Sorry, I didn't spend enough time explaining. Right now, we have reasonably elaborate dispatch logic between different versions of a compiled function, based on shapes, which arguments are supplied, etc. This flag would assert that only a single version should ever exist, and calling the compiled function a second time with mismatched arguments would be an error. Does that make sense?
kaushikcfd commentedon Apr 26, 2023
Yep, makes sense. Thanks! Essentially this flag should ensure that length of
BaseLazilyCompilingFunctionCaller.program_cacheshould not exceed 1.inducer commentedon Apr 26, 2023
Yup, exactly.
majosm commentedon Apr 26, 2023
Question: Would the
allow_single_version=Truecase check if the arguments are the same as when it was compiled? Or is it more of an "at your own risk" thing?kaushikcfd commentedon Apr 26, 2023
It should be an error at this point if
len(self.program_cache) > 1.majosm commentedon Apr 26, 2023
Does it still incur the "dispatch cost to select which implementation to use" then?
kaushikcfd commentedon Apr 26, 2023
Aah fair. I missed that as a constraint. Though I am unsure how to avoid the dispatch cost since we would always need to compute the
arg_id_to_descr.majosm commentedon Apr 26, 2023
In any case, I can see the value in avoiding multiple compiles. On the user side, though, I'm not sure if it's always obvious enough when it's happening that one would know to intervene. Kind of makes me wonder if the
single_version_only=Truebehavior should be the default? i.e., emit an error if it tries to compile multiple times, unless the caller explicitly says it's OK. Not sure how much stuff this would break, though.inducer commentedon Apr 26, 2023
At the very least, it would break the LSRK45 time stepper, which relies on having two different versions at the moment.
https://github.com/inducer/grudge/blob/c9906c9a20bfb5c78733093ccba716d7ac036897/grudge/shortcuts.py#L44-L62
matthiasdiener commentedon Apr 26, 2023
How about just emitting a warning instead of an exception? This would even work without necessarily changing the API.
inducer commentedon Apr 26, 2023
IMO, both multi-version and single-version are legitimate uses of the function, unless the user says otherwise.