Skip to content

Two bugfixes, including adbuerger/pycombina#27#33

Open
mirhahn wants to merge 1 commit intoadbuerger:developfrom
mirhahn:issues/upstream#27
Open

Two bugfixes, including adbuerger/pycombina#27#33
mirhahn wants to merge 1 commit intoadbuerger:developfrom
mirhahn:issues/upstream#27

Conversation

@mirhahn
Copy link
Copy Markdown
Contributor

@mirhahn mirhahn commented Nov 20, 2023

This pull request appears to fix the max_up_time issue #27. It modifies CombinaBnBSolver::compute_child_node_properties so that up_time_child[b_active_parent] is reset to 0.0 when the control switches and the parent control exists (i.e., the node is not root).

I had to reverse-engineer and guess what the semantics of some of the variables are. The result appears to pass all tests, but since this is a very central method, this should be scrutinized more heavily. I have also added some comments with my best guesses as to what the code does. Maybe some of these can be retained.

During debugging, I have also noticed some debugging output that does not lock the GIL, thereby crashing the solver if compiled with debugging symbols. I have also fixed that.

Note: The additional test case has been squashed out of the branch. I am unsure as to whether it would cause licensing issues because it is derived from @jelgerjansen's reproduction instructions. The new version does not include anything other than my own code patches.

The unnamed bug occurs only when the extension module is compiled with
debugging flags. In this case, the final stat output would use Python's
print function without acquiring the Global Interpreter Lock, which would
trigger an error in Python (or potentially crash if Python is compiled
without debugging flags).

Note:
  The issue-specific test has been removed to avoid licensing issues.
@mirhahn mirhahn marked this pull request as ready for review November 26, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant