-
Notifications
You must be signed in to change notification settings - Fork 47
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
Improve dynamic changing of SETTINGS
parameters for decorated functions with active workflow engines
#2147
Comments
I've tried a workflow similar to your suggestion. I wasn't sure how to use def change_settings_wf(func: Callable, settings: Dict, decorator: Callable) -> Callable:
from quacc import change_settings
def concatenated(*args, **kwargs):
with change_settings(settings):
return func.func(*args, **kwargs)
return decorator(concatenated)
from quacc import job
relax_job = change_settings_wf(relax_job, {"DEBUG": True}, job) I think it would be great if I could directly get the |
Thanks for the suggestion! We can replace from quacc import strip_decorator
return strip_decorator(func)(*args, **kwargs) So that it works for any workflow engine. If you'd wish to do so, feel free to open a PR and add a test for Parsl! I can add the other tests. Alternatively, if you do not have the time or desire to do so, I can work on implementing this soon.
Definitely. This was also a problem in #1865. I tried to solve it recently by appending a
Not a huge fan of that approach but perhaps it's the only way. I'll keep thinking on this. |
No problem! Sure, I can work on it tomorrow since it's getting late here. But of course you can go ahead if you would like to implement it sooner. I see, that sounds good. It seems there's no way to know the wrapper from the wrapped function without any modification. I agree that this is the only way. |
It's no rush from my side. I probably won't have time to seriously devote to this until the middle of next week. Thanks for kicking it off though! Shouldn't be too painful to implement. |
No problem, just opened a PR. I only added a unit test for Parsl. FYI, I found that there can be a potential issue in the nested call of static_job1 = change_settings_wf(static_job, {"RESULTS_DIR": "/new/path1"}, job)
static_job2 = change_settings_wf(static_job1, {"RESULTS_DIR": "/new/path2"}, job)
#static_job in static_job2 will have "RESULTS_DIR" of "/new/path1" This can be problematic when using flow or subflow that calls jobs in it. At the moment, I don't know how to fix it. We might need another approach for change_settings. |
…low engine (v2) (#2277) This PR is a collaboration with @honghuikim. It is a continuation of #2163 that addresses changes introduced in #2276. For additional history on this PR, please refer to #2163. The focus of this PR is to enable dynamic changing of Pydantic model settings when using an active workflow engine. This is achieved, in large part, through a new `settings_swap` keyword argument to the decorator. Closes #2147. --------- Co-authored-by: Honghui Kim <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Honghui Kim <[email protected]>
What new feature would you like to see?
Recently, we introduced a function
change_settings
that can be used to temporarily modify the settings of some function calls in a script. See here for details. An example is like that shown below:This works wonderfully. However, as noted on that same page, if you dispatch the calculation to a remote machine (as is common with the use of a workflow engine), there is no knowledge of changes to in-memory parameters like the global
SETTINGS
object. We should come up with a smart and user-friendly way to address this.For instance, let's say you want to change
SETTINGS.DEBUG
toTrue
for a@job
calledrelax_job
. Most likely, it can be solved by:@job
,@flow
, or@subflow
decorator (from quacc import strip_decorator
) from the imported function. In this case, it'd be a@job
.SETTINGS.DEBUG
inside of the function definition. Important: we must retain the original function's name, signature, type hints, etc. so presumablyfunctools.wraps
here.The tools to do this are likely already in
quacc.wflow_tools.customizers
, but the exact logic to get it right in an automated fashion currently escapes me.For background context, refer to #2145 and my answer therein.
The text was updated successfully, but these errors were encountered: