Improve exception-handling in SacessOptimizer #1517
Conversation
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #1517 +/- ##
===========================================
- Coverage 82.80% 82.78% -0.03%
===========================================
Files 163 163
Lines 13962 14007 +45
===========================================
+ Hits 11561 11595 +34
- Misses 2401 2412 +11 ☔ View full report in Codecov by Sentry. |
Previously, uncaught exceptions in SacessOptimizer worker processes would have resulted in deadlocks in `SacessOptimizer.minimize`. These changes will (in most cases) prevent deadlocks and other errors due to missing results from individual workers. Furthermore, this will lead to termination of minimize() relatively soon after an error occurred on some worker - not only after some other exit criterion is met. Closes ICB-DCM#1512.
dweindl
force-pushed
the
1512_sacess_errs
branch
from
4b79df7 to
217b380
Compare
| return worker.run(problem=problem, startpoint_method=startpoint_method) | ||
| return worker.run(problem=problem, startpoint_method=startpoint_method) | ||
| except Exception as e: | ||
| with suppress(Exception): |
There was a problem hiding this comment.
why do we need to suppress the exception here since we are already in an except? Or do you want to make sure it is definitely aborting?
There was a problem hiding this comment.
It the logging fails for what ever reason, we don't care, but we need to ensure that the line below that block gets executed.
Previously, uncaught exceptions in SacessOptimizer worker processes would have resulted in deadlocks in
SacessOptimizer.minimize.These changes will (in most cases) prevent deadlocks and other errors due to missing results from individual workers. Furthermore, this will lead to termination of minimize() relatively soon after an error occurred on some worker - not only after some other exit criterion is met.
Closes #1512.