Repository navigation
Extend external LammpsLibrary interface - #237
Merged
Merged
Conversation
prabhath-c
added a commit
to prabhath-c/calphy
that referenced
this pull request
Sep 7, 2026
The previous commit tracked session ownership inside LibraryRunner
itself (_owns_lmp, checked in close(), with clear() issued at
construction time). Same outcome, wrong layer: LibraryRunner had to
know something that's really a property of the job, not the runner.
Move the decision to Phase.lammps_close, the method every driver stage
already calls when it's done with its runner for that stage:
def lammps_close(self, lmp):
if self.lmp is not None and self.calc.execution_mode == "library":
lmp.clear()
else:
lmp.close()
self.lmp (set once in Phase.__init__, reused across every stage) is
exactly "was an external session ever supplied to this job" -- the
execution_mode check guards the one edge case where it wouldn't apply
(an lmp= passed with execution_mode="executable" is never touched by
create_object, so there is nothing to reuse). LibraryRunner goes back
to being a dumb wrapper: close() always closes, no ownership tracking.
Added clear() as its per-stage counterpart, called from lammps_close
instead of at construction -- the same effect, timed like the actual
end of a stage rather than the start of the next one.
Based on the same pattern from ICAMS#237 (jan-janssen,
"Extend external LammpsLibrary interface"), which solved this exact
problem the same way on the pre-refactor codebase before pylammpsmpi
support was removed and later re-added without carrying this over.
Tests reworked to match: dropped the _owns_lmp-specific ones, added
coverage for LibraryRunner.clear() directly and for lammps_close's
three branches (external+library -> clear, own session -> close,
external+non-library -> close), plus the same multi-stage reuse
regression test, now driven through job.lammps_close(...) instead of
calling LibraryRunner.close() directly.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After some additional testing I noticed that the default setting of closing the LAMMPS library after a given simulation task is confusing when using an external LammpsLibrary interface. So in this pull request I propose to just clear the state of the library rather than closing the library interface completely. Still this comes at the cost that the user has to close the library at the end.