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

Add write_input() before transferring files to remote #1511

Merged
merged 10 commits into from
Jul 5, 2024

Conversation

jan-janssen
Copy link
Member

@jan-janssen jan-janssen commented Jul 3, 2024

This pull request addresses the issue raised by @Leimeroth in pyiron/pyiron_atomistics#1471 that the remote submission for LAMMPS jobs failed after the switch to using the calculate() function. While it is not exactly clear why the VASP job works fine, the work around in this branch fixes the issue for LAMMPS.

@Leimeroth
Copy link
Member

Ok, so check if working directory is empty leads to a bunch of test failures and some horrible side effects, because I created the job with delete_existing_job=True. However the contents of the working directory are not deleted on time,
so on my local computer no input is written with the check.

@jan-janssen
Copy link
Member Author

Ok, so check if working directory is empty leads to a bunch of test failures and some horrible side effects, because I created the job with delete_existing_job=True. However the contents of the working directory are not deleted on time, so on my local computer no input is written with the check.

The != and == sign were mixed up. You want to write the input files when the directory is empty and do not write them when it is not empty.

@jan-janssen jan-janssen marked this pull request as ready for review July 5, 2024 06:13
@jan-janssen
Copy link
Member Author

@Leimeroth Can you run your tests again, like submitting a job with a custom potential to a remote cluster? Once this works and you are fine with the changes as they are in this pull request, can you approve this pull request? Then I can make sure it is included in the release of the next version. I aim to update both pyiron_base and pyiron_atomistics before the pyiron meeting on Monday.

@Leimeroth
Copy link
Member

Leimeroth commented Jul 5, 2024

Now it works with my own potential and the EAM.
However, only the check prevents it from rewriting input, which could cause trouble when trying to rerun jobs.
When I run them again using delete_existing_job=True on my local computer the old files are still present on the remote cluster, so they will be used instead of my updated inputs.

@jan-janssen
Copy link
Member Author

Are you sure about this? I thought the files are written locally and then transferred. So as the job is first deleted locally before the new input files are written the working directory is removed and a new empty working directory is created. Then at the copying stage we do not use the write_input() function so it should still overwrite the files on the cluster.

@Leimeroth
Copy link
Member

Are you sure about this? I thought the files are written locally and then transferred. So as the job is first deleted locally before the new input files are written the working directory is removed and a new empty working directory is created. Then at the copying stage we do not use the write_input() function so it should still overwrite the files on the cluster.

You are right, I just tested it. Fine to merge from my side.

@jan-janssen jan-janssen merged commit d9bf802 into main Jul 5, 2024
25 checks passed
@jan-janssen jan-janssen deleted the remote_submission branch July 5, 2024 06:52
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 this pull request may close these issues.

None yet

2 participants