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

Fixes verbosity flags not being respected in GLPK #49

Conversation

ManasJayanth
Copy link

@ManasJayanth ManasJayanth commented Dec 18, 2023

Recently I noticed logs from GLPK backend on MacOS Azure Pipelines runner. This PR adds verbosity flag checks so that the verbosity level is respected everywhere.

TBH, I'm not so sure why this wasn't discovered earlier. Something to do with Apple/Clang?

Lately, on macos, we have been seeing the following logs despite
keeping verbosity level to 0

```
Constructing initial basis...
Number of 0-1 knapsack inequalities = 4695
Constructing conflict graph...
Conflict graph has 1691 + 294 = 1985 vertices
```

This patch correctly sets the env->term_out according the verbosity params
These changes were made for local development
@ManasJayanth
Copy link
Author

Running some tests here: DiningPhilosophersCo/opam#1

@kit-ty-kate
Copy link
Contributor

After a few hours of debugging, I found the underlying issue:


does nothing on macOS since macOS 12.7.1 / 13.6.3 / 14.2 for some reason.
I’ll report the issue to Apple and see what they say.

In the meantime fclose(stdout) seems to work so as a quickfix I’d prefer to go this way instead. I’ll open this PR. We should probably clean up this code later, this is a bit of a mess. I’m guessing this code come from the need of modifying glpk as little as possible as it is still being developed.

@kit-ty-kate
Copy link
Contributor

#50

@ManasJayanth
Copy link
Author

If it's okay, I'm going to close this PR as the alternative is much cleaner

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.

2 participants