-
Notifications
You must be signed in to change notification settings - Fork 89
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
Add verbosity flag for MPB #119
Conversation
(Still needs fine-tuning)
The Travis failure on MacOS seems to be new? Not sure what is going on, since it seems like a convergence problem, but this PR shouldn't have changed anything except for output… maybe the compiler or library versions changed? |
I'm looking into the test failure now. |
The tests pass for me on my OSX machine, so it may be that something has changed on Travis. |
The problem was indeed with Travis. After restarting just the failing macOS test, it's passing now. |
@@ -93,12 +94,12 @@ double linmin(double *converged_f, double *converged_df, | |||
} | |||
|
|||
if (*task != 'C') { /* not converged; warning or error */ | |||
if (verbose || *task == 'E') | |||
if ((verbose || mpb_verbosity >= 2 || *task == 'E') && mpb_verbosity >= 2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this have been mpb_verbosity >= 3
? Same for line 102.
Right now, this gets spammed multiple times for every k-point when calling mpb, which seems undesirable as default behavior.
This seems to have added some I-think-unintended spam to the ordinary operation of MPB from the command line (see e.g. the CI logs: https://travis-ci.org/github/NanoComp/mpb/jobs/729108444#L4730) I've indicated what I think the issue is above; if either @RobinD42 or @stevengj can confirm this is the right fix, I can submit a PR. |
Yes, there is still some adjustments that need to be made when checking the verbosity level. To keep things simpler when migrating to this new scheme I just checked for 2 everywhere. Previously most things were unconditional, or were based on a boolean flag, so the first pass was intended mainly to switch to the new verbosity flags, with the fine-tuning to happen later. Any help or suggestions for that fine-tuning is welcome. |
Please feel free to submit a PR to make the default less verbose. |
Fixes #116
Note that since there is also mpb code (python wrappers and such) in the Meep lib then there will be another PR there that is part of this change.