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

WIP: Fix deadlock problems around running forward model callback in subprocess #5974

Conversation

valentin-krasontovitsch
Copy link
Contributor

@valentin-krasontovitsch valentin-krasontovitsch commented Aug 25, 2023

Issue
Resolves #5952

Approach
we change the process of making a new process from forking to spawning. furthermore, we add a bunch of guards for not writing already existing / uptodate files during init of local storage things. we also needed to fix some issues with pickling for the process spawning.

Pre review checklist

  • TODO Author has read through code and done QC
  • Added appropriate release note label
  • PR title captures the intent of the changes, and is fitting for release notes.
  • NEEDS REBASE!! Commit history is consistent and clean, in line with the contribution guidelines.
  • IRRELEVANT - Updated documentation
  • Ensured new behaviour is tested

@valentin-krasontovitsch valentin-krasontovitsch changed the title Fix subprocess WIP: Fix subprocess Aug 25, 2023
@valentin-krasontovitsch valentin-krasontovitsch added the release-notes:skip If there should be no mention of this in release notes label Aug 25, 2023
@valentin-krasontovitsch valentin-krasontovitsch changed the title WIP: Fix subprocess WIP: Fix deadlock problems around running forward model callback in subprocess Aug 25, 2023
@JHolba JHolba force-pushed the fix-subprocess branch 3 times, most recently from ab4e910 to ac51cf1 Compare August 28, 2023 07:46
@codecov-commenter
Copy link

Codecov Report

Merging #5974 (32d3d55) into main (637f843) will increase coverage by 0.02%.
The diff coverage is 88.99%.

@@            Coverage Diff             @@
##             main    #5974      +/-   ##
==========================================
+ Coverage   81.90%   81.93%   +0.02%     
==========================================
  Files         352      352              
  Lines       21896    21926      +30     
  Branches      811      811              
==========================================
+ Hits        17934    17964      +30     
  Misses       3673     3673              
  Partials      289      289              
Files Changed Coverage Δ
src/ert/config/ert_config.py 95.15% <ø> (ø)
src/ert/job_queue/queue.py 87.98% <ø> (ø)
src/ert/callbacks.py 89.61% <78.78%> (-5.71%) ⬇️
src/ert/storage/local_experiment.py 94.36% <87.50%> (-1.02%) ⬇️
src/ert/storage/local_storage.py 91.35% <89.47%> (-0.79%) ⬇️
src/ert/job_queue/job_queue_node.py 85.83% <90.47%> (-1.32%) ⬇️
src/ert/config/__init__.py 100.00% <100.00%> (ø)
src/ert/config/ensemble_config.py 95.93% <100.00%> (ø)
src/ert/config/gen_kw_config.py 98.06% <100.00%> (+0.01%) ⬆️
src/ert/config/parsing/context_values.py 90.90% <100.00%> (+0.28%) ⬆️
... and 8 more

... and 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@xjules
Copy link
Contributor

xjules commented Oct 2, 2023

The code has been reverted; therefore no more deadlock issue.

@xjules xjules closed this Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:skip If there should be no mention of this in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deadlock due to subprocessing
4 participants