-
Notifications
You must be signed in to change notification settings - Fork 15
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
Introduce calculate() function #1477
Conversation
for more information, see https://pre-commit.ci
…nto executablecontainer_runstatic
…lecontainer_runstatic # Conflicts: # pyiron_base/jobs/job/runfunction.py
for more information, see https://pre-commit.ci
…nto executablecontainer_runstatic
I start by switching the |
…_runstatic' into scriptjob_calculate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit annoying that this now duplicates a lot of the logic present in GenericJob.run_static and execute_job_with_external_executable
. Do you see an option to unify this again (in the future)?
Co-authored-by: Marvin Poul <[email protected]>
for more information, see https://pre-commit.ci
…nto executablecontainer_runstatic
…_runstatic' into scriptjob_calculate
for more information, see https://pre-commit.ci
Scriptjob implement calculate() function
I updated the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except for my minor comment I'm ok with this.
…et_output_parameter_dict()
…lecontainer_runstatic
for more information, see https://pre-commit.ci
…_runstatic' into executablecontainer_runstatic # Conflicts: # pyiron_base/jobs/job/runfunction.py
for more information, see https://pre-commit.ci
…nto executablecontainer_runstatic
@@ -122,5 +122,5 @@ class PythonTemplateJob(TemplateJob): | |||
|
|||
def __init__(self, project, job_name): | |||
super().__init__(project, job_name) | |||
self._python_only_job = True | |||
self._job_with_calculate_function = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't this will actually work for PythonTemplateJob
, because the CalculateCaller
is now expected to run a script and won't touch run_static
(which is what implementors are supposed to overload), right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As run_static()
is defined in PythonTemplateJob
it overwrites the run_static()
in GenericJob
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like I wrote, I don't think this will work with the TemplateJob
, but it also doesn't need to be unified now. Keeping _python_only_job
around and checking for both in _check_if_input_should_be_written
should be sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, I missed that it's overloaded anyway in case of TemplateJob.
In the previous interface a new class could be implemented in pyiron by defining two functions
write_input()
andcollect_output()
. The example class would look like this:The new interface replaces the
write_input()
function with anget_input_parameter_dict()
function which returns thefiles_to_write
and thefiles_to_copy
as a dictionary. In addition, thecollect_output()
function is placed outside the job class. Both of these changes are designed to support a more functional approach inpyiron_base
. Only theget_input_parameter_dict()
andget_output_parameter_dict()
functions are now part of the new job class. Theget_input_parameter_dict()
function takes the input defined by the user on different properties likejob.input.structure
or functions likejob.set_kpoints()
and merges everything into a dictionary with input file content. This dictionary of input file content allows expert users to overwrite the pyiron generated input with their own input, if theget_input_parameter_dict()
function defined in the new job class supports this. In the same way theget_output_parameter_dict()
function allows the developer to provide additional parameters to thecollect_output()
function:For those users who only want define the
write_input()
andcollect_output()
function,pyiron_base
still supports thewrap_executable()
function:The limitations of the
wrap_executable()
function are: MPI parallelism is not supported and theinput_dictionary
is directly communicated to thewrite_input()
function. So it is not possible to have a generic interface and an expert user interface with this approach. Internally thewrap_executable()
already uses the new job class interface. In the same way thecollect_output()
function only accepts theworking_directory
as input parameter and it is currently not possible to provide additional parameters for the parsing of the output.