Issue 340: Add a prefix to benchmarks #414
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Try this Pull Request!Open Julia and type: import Pkg
Pkg.activate(temp=true)
Pkg.add(url="https://github.com/CDCgov/Rt-without-renewal", rev="partial-issue340", subdir="EpiAware")
using EpiAware |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #414 +/- ##
=======================================
Coverage 93.57% 93.57%
=======================================
Files 54 54
Lines 560 560
=======================================
Hits 524 524
Misses 36 36 ☔ View full report in Codecov by Sentry. |
damonbayer
reviewed
Jul 31, 2024
|
Annoyingly it looks like we might not get any transcient gradient warnings on this go which makes checking this really is working a bit of a pain! |
Doesn't seem wise to approve without a demonstration of it working in CI. Can you force it to warn and then fix it so we see a demonstration? |
Benchmark resultJudge resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTargetBaselineTarget resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoBaseline resultBenchmark Report for /home/runner/work/Rt-without-renewal/Rt-without-renewalJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoRuntime information
|
|
urggh checking... also it doesn't work :(. I think that this means it is calling the check after the suite has been constructed i.e in benchmarktools which is why wrapping |
|
So running this locally it catches the depreciation warnings as you might expect but that showed no gradient warnings (the target of interest). Running that suite now to see if this is where they show up (vs when building the suite which was how I thought it was set up to work). I'm using this as I think this might be where gradient issues live but will also try some of the other submodules (this is worth doing anyway as it helps inform #340) |
|
So these are rendered stochastically when you make the suite and they are happening in the obs model (which is good to know for the future). It seems like they aren't actually warnings as expected hence not being caught |
|
Hmm weird but they definitely appear to be based on the output of the CI
|
|
Okay so locally I am seeing these (when they happen): Warnings from Model{typeof(generate_observations), (:obs_model, :y_t, :Y_t), (), (), Tuple{NegativeBinomialError{HalfNormal{Float64}}, Vector{Float64}, Vector{Float64}}, Tuple{}, DefaultContext}(EpiAware.EpiAwareBase.generate_observations, (obs_model = NegativeBinomialError{HalfNormal{Float64}}(HalfNormal{Float64}(μ=0.01)), y_t = [10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0], Y_t = [10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.0]), NamedTuple(), DefaultContext()):
┌ Warning: There is disagreement in the log-density values!
└ @ TuringBenchmarking ~/.julia/packages/TuringBenchmarking/0wcdT/src/TuringBenchmarking.jl:268
┌ Warning: There is disagreement in the gradients!
└ @ TuringBenchmarking ~/.julia/packages/TuringBenchmarking/0wcdT/src/TuringBenchmarking.jl:275
┌ Warning: There is disagreement in the log-density values!
└ @ TuringBenchmarking ~/.julia/packages/TuringBenchmarking/0wcdT/src/TuringBenchmarking.jl:268
┌ Warning: There is disagreement in the gradients!
└ @ TuringBenchmarking ~/.julia/packages/TuringBenchmarking/0wcdT/src/TuringBenchmarking.jl:275So that is good. I think they aren't showing in the Ci as the thing throwing the warning there is Also this is a good indication that this functionality is helping as we thought this implementation might be a problem but were not quite sure. |
This was referenced Aug 1, 2024
seabbs
commented
Aug 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR partially addresses #340 by adding a prefix to any warnings produced by
make_turing_suitebased on the inputmodel. This should help us locate potential issues more easily.@damonbayer if you have time to review this that would be great. We are looking at the output of the
benchmarkCI to see if it has worked.You can run the
prefix_warningswrapper using this example/In principle yes the code in benchmarks should also have tests but I think we can leave for now as I am hoping that the Turing devs put something better than this in upstream.