Repository navigation
Close SQLite log connections when minimize returns - #714
Open
hmgaudecker wants to merge 2 commits into
Open
hmgaudecker wants to merge 2 commits into
hmgaudecker wants to merge 2 commits into
Conversation
The log stores kept pooled SQLite connections open until the OptimizeResult was garbage collected. In WAL mode, the database file therefore changed after minimize returned (checkpoint on close), which breaks tools that hash outputs right after a task (e.g. pytask re-runs the task every time). - Close the logger's engines at the end of optimization. - Dispose of the engines used for table reflection and after reads, so a LogReader does not hold the file open either. Closes optimagic-dev#713. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y3CuwvNUhYTSpriKYSwqmq
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y3CuwvNUhYTSpriKYSwqmq
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
This branch has not been deployed
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
1 participant
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.
Closes #713.
Problem
The SQLite log stores keep pooled connections open until the
OptimizeResultis garbage collected. In WAL mode,log.dbtherefore still changes afterminimizereturns (the WAL is checkpointed into the file when the last connection closes). Tools that hash outputs right after a function returns see a different file than the one that ends up on disk; e.g. pytask re-runs every estimation task that writes an optimagic log.Changes
LogStore.close()(no-op default; the SQLite store disposes of its engines).minimize/maximizecall it after the optimization, before creating theLogReader.SQLAlchemyConfig.metadataand of each store's engine after table reflection.LogReader(e.g.res.logger.read_history()) does not hold the file open. Reads during the optimization are rare (step bookkeeping only), so pooling for the hot write path is kept.I first tried
NullPool; it doubled the logging overhead (Nelder–Mead, 2000 logged evaluations: 1.08 s vs 0.52 s on main). With this PR: 0.50–0.54 s.Tests
Two new tests in
tests/optimagic/logging/test_logger.py; both fail on main (alog.db-walfile exists afterminimizereturns / after reading the history).tests/optimagic/logging tests/optimagic/optimization tests/estimagic: 1426 passed, 11 skipped.I did not add a
CHANGES.mdentry since entries reference the PR number; happy to add one.https://claude.ai/code/session_01Y3CuwvNUhYTSpriKYSwqmq