PyPSA / linopy

Linear optimization with N-D labeled arrays in Python
https://linopy.readthedocs.io
MIT License
155 stars 43 forks source link

Retrieve solution before closing the gurobi env #193

Closed dannyopts closed 9 months ago

dannyopts commented 9 months ago

Currently the Gurobi env is closed before the solution has been retrieved and this can result in an exception being thrown based on a race condition (described in the issue referenced below)

Fixes https://github.com/PyPSA/linopy/issues/192

codecov[bot] commented 9 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (7e76a67) 89.22% compared to head (dc7c996) 89.25%. Report is 5 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #193 +/- ## ========================================== + Coverage 89.22% 89.25% +0.03% ========================================== Files 15 15 Lines 3136 3182 +46 Branches 723 731 +8 ========================================== + Hits 2798 2840 +42 - Misses 234 237 +3 - Partials 104 105 +1 ``` | [Files](https://app.codecov.io/gh/PyPSA/linopy/pull/193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PyPSA) | Coverage Δ | | |---|---|---| | [linopy/solvers.py](https://app.codecov.io/gh/PyPSA/linopy/pull/193?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PyPSA#diff-bGlub3B5L3NvbHZlcnMucHk=) | `90.10% <88.88%> (+0.16%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dannyopts commented 9 months ago

Does the fact coverage has gone down mean we cant merge this PR?

It seems to me this is not a change that would require a test, but if it would normally be accompanied by a test, can someone give me a steer what that would look like?

FabianHofmann commented 9 months ago

Thanks @dannyopts, the diffs are a bit hard to read. But it seems you intended the block which was not in the with block, is that right? I would not worry about the code coverage of the patch.

dannyopts commented 9 months ago

@FabianHofmann yes exactly, we dont exit the stack until after we have read the solution so the gurobi env is not destroyed.

FabianHofmann commented 9 months ago

great, thanks!