Skip to content

Ignore stdout on refresh=0#324

Merged
jgabry merged 6 commits intomasterfrom
zero_refresh_fix
Oct 27, 2020
Merged

Ignore stdout on refresh=0#324
jgabry merged 6 commits intomasterfrom
zero_refresh_fix

Conversation

@rok-cesnovar
Copy link
Copy Markdown
Member

@rok-cesnovar rok-cesnovar commented Oct 25, 2020

Submission Checklist

  • Run unit tests
  • Declare copyright holder and agree to license (see below)

Summary

The newton optimization method does not respect the refresh arg. This PR ignores all stdout when refresh = 0, which seems logical to me as the only reason someone would use refresh = 0, is when they do not want any info printed.

Copyright and Licensing

Please list the copyright holder for the work you are submitting
(this will be you or your assignee, such as a university or company):
Rok Češnovar, Univ. of Ljubljana

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses:

@jgabry
Copy link
Copy Markdown
Member

jgabry commented Oct 26, 2020

Yeah ignoring std_out if refresh=0 makes sense to me. Looks like tests are failing, but otherwise this seems good to me.

@rok-cesnovar
Copy link
Copy Markdown
Member Author

Looks like tests are failing, but otherwise this seems good to me.

Yeah, I touched the existing show_messages arg. Internally we now use show_stderr_messages and show_stdout_messages in procs. show_stderr_messages is what show_messages did previously and show_stdout_messages is used for this fix.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 26, 2020

Codecov Report

Merging #324 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #324      +/-   ##
==========================================
+ Coverage   89.23%   89.24%   +0.01%     
==========================================
  Files          12       12              
  Lines        2535     2538       +3     
==========================================
+ Hits         2262     2265       +3     
  Misses        273      273              
Impacted Files Coverage Δ
R/model.R 89.52% <100.00%> (+0.03%) ⬆️
R/run.R 99.58% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa67022...1f74a5e. Read the comment docs.

@jgabry jgabry merged commit ea13eba into master Oct 27, 2020
@jgabry jgabry deleted the zero_refresh_fix branch October 27, 2020 00:16
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.

$optimize() prints output for every iteration when refresh = 0 and algorithm = newton

3 participants