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

Upgrade pyiron_base to 0.6.14 #1265

Merged
merged 2 commits into from
Dec 21, 2023
Merged

Upgrade pyiron_base to 0.6.14 #1265

merged 2 commits into from
Dec 21, 2023

Conversation

jan-janssen
Copy link
Member

No description provided.

@jan-janssen
Copy link
Member Author

Waiting for conda-forge/pyiron_base-feedstock#180

@jan-janssen jan-janssen reopened this Dec 19, 2023
@jan-janssen
Copy link
Member Author

@pmrv There seems to be an issue with the new read-only objects pyiron/pyiron_base#1162

======================================================================
ERROR: test_multiple_jobs (testing.test_serialMaster.TestSerialMaster.test_multiple_jobs)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/pyiron_atomistics/pyiron_atomistics/tests/testing/test_serialMaster.py", line 66, in test_multiple_jobs
    job_ser_reload.append(ham)
  File "/usr/share/miniconda3/envs/my-env/lib/python3.11/site-packages/pyiron_base/jobs/master/generic.py", line 171, in append
    self.server.cores = job.server.cores
    ^^^^^^^^^^^^^^^^^
  File "/usr/share/miniconda3/envs/my-env/lib/python3.11/site-packages/pyiron_base/interfaces/lockable.py", line 54, in dispatch_or_error
    raise Locked(
pyiron_base.interfaces.lockable.Locked: Object is currently locked!  Use unlocked() if you know what you are doing.

======================================================================
ERROR: test_convergence_goal (testing.test_serialMaster_non_modal.TestSerialMaster.test_convergence_goal)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/pyiron_atomistics/pyiron_atomistics/tests/testing/test_serialMaster_non_modal.py", line 90, in test_convergence_goal
    self.project.wait_for_job(job_ser, interval_in_s=5, max_iterations=50)
  File "/usr/share/miniconda3/envs/my-env/lib/python3.11/site-packages/pyiron_base/project/generic.py", line 1457, in wait_for_job
    wait_for_job(
  File "/usr/share/miniconda3/envs/my-env/lib/python3.11/site-packages/pyiron_base/jobs/job/extension/server/queuestatus.py", line 237, in wait_for_job
    raise ValueError(
ValueError: Maximum iterations reached, but the job was not finished.

----------------------------------------------------------------------

@pmrv
Copy link
Contributor

pmrv commented Dec 19, 2023

It seems the error originates here where an already run master job has new jobs appended and is re-run. I would actually argue that this is exactly the sort of problem read-only objects are supposed to catch. I could fix this in base, but I really think the test is wrong here in atomistics and should be changed. Thoughts?

@pmrv
Copy link
Contributor

pmrv commented Dec 19, 2023

So from what I have reconstructed so far the test actually needs to be written in the original way, because calling run on SerialMaster only runs the last appended job, possibly in a restart loop if a convergence goal is set. This appears to be by design. The only actual implementations ConvEncutSerial/ConvergenceVolume also just restarts the same job. I have no idea what this test is even supposed to check.

I've opened a quick fix in base, however as far as I can tell these two classes are the only two serial masters in existence (create_pipeline uses FlexibleMaster), so I'd actually prefer to simply remove both classes here and SerialMasterBase from base entirely.

@niklassiemer
Copy link
Member

I would in this case not release a new version. Even if we manage to fix the issue and release before christmas, I will not apply it on the cluster before christmas.

@pmrv
Copy link
Contributor

pmrv commented Dec 20, 2023

I would prefer to have a version available that has #1249. I'd say we release the current HEAD (without this PR) and install that on the cluster, that means for now pyiron_base will still be 0.6.13 on the cluster, but I don't see a problem with that.

@pmrv pmrv added the integration Start the notebook integration tests for this PR label Dec 20, 2023
@pmrv pmrv merged commit bb93280 into main Dec 21, 2023
28 checks passed
@delete-merged-branch delete-merged-branch bot deleted the pyiron_base branch December 21, 2023 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration Start the notebook integration tests for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants