Synchronize feature branch - #238
Merged
Merged
Conversation
This most important change was the removal of the classifier that specified the license, which was causing tools to emit a license deprecation warning.
Reran script to confirm all tests still passing with no visualizations to close. Progress!
Failing as expected.
Based on lessons learned from this bug, we now explicitly use loc/scale argument names. With this, the test suite runs through successfully. The visualizations and test output for the new tests now look good. At least they are similar to results for the MH/Normal step tests.
Not only does this test allow us to stress test the uniform step distribution in MH is more than 1D, but it also allows us to confirm that our test tool does work on 4D problems (albeit with boring target distributions). My tests all passed and the visualizations/logging for the new test look good.
All tests now running regression tests and passing.
Fix MH/uniform step distribution bug
Confirm that my test suite runs through successfully with no visualizations.
Most importantly, this moves the current if/else block outside the loop. We are also sampling the full vector in a single rvs call and computing the proposed step in vectorized fashion. An additional benefit of this is that we create the proposal distribution once as a frozen distribution and draw from that at each loop iteration, which reflects the static nature of the proposal distribution in the algorithm. Prior to these changes, the test suite ran in 167 seconds. With this change, the test suite runs through successfully with bitwise identical results (as expected) and in 131 seconds. Not a massive speedup, but I'll take it!
Optimize MH Loop
Previous changes to the computation/use of p_accept were leading to overflow warnings being posted by the official package test suite. They were not, however, showing up with the test script's suite. This commit expands out the handling of how to choose the next theta point by precluding the possibility of overflow and explaining how underflow can happen and how the algorithm deals with it. See comments in code for more benefits. Official unit test suite and unofficial test script suite passing with no warnings. Since the new implementation is no longer drawing from Bernoulli with p_accept=1, the results are different from previous benchmarks. After updating benchmarks to the new results, a rerun of the test script ran through successfully. Curiously, despite expanding the if/elif/else block within the MCMC loop, performance appears to have improved slightly.
This is necessary to verify that all new results are sufficiently good for converting into the next set of benchmarks.
Explicitly manage over/underflows in MH
Prefer initializing arrays to NaN so that any bugs in the code that result in insufficient setting of all elements in the array will hopefully result in someone finding NaNs. The first few tests ran through with identical results, as expected. However, one test did fail with the new error since it was intentionally designed with a start distribution that is disjoint from the support of the target distribution. The test suite will be fixed in a subsequent commit so that reviewers can check out this commit to confirm correct detection and reporting of failure.
Any draw from the start distribution should now yield a positive density. Note that after fixing this, the test still failed since the new results were started from a new starting point. After saving the new result as the new benchmark, a rerun of the full test suite finished successfully.
First evaluation must yield positive density
This new version is causing issues, so I temporarily disallow it so that we can proceed with unrelated development activities. Running `tox -r -e nocoverage` before and after making this change revealed that I could locally reproduce the related issues and avoid them with this change.
We again have a full set of benchmarks that should be sufficient for upcoming refactoring work.
I set the burn-in rate to 100,000 for the Uniform4D test to see a clear manifestation of the logging errors. After making this change, that test runs through with reasonable logging results. The test is successful because the logging bug did not affect the execution of the sampling nor the computation of the final acceptance rate.
Fix MH logging
Now the configuration is not tied just to MH. This percolated throughout the test script code so that we open the door to testing LMC and PTLMC. Test script ran through with no issues and the unit tests run through cleanly as well.
The results do not look good and I don't see any parameters for me to adjust. This fits with the fact that LMC is currently listed as research grade. This second set of tests fail because the sampler cannot yet be made deterministic. That will be done on a later branch.
Homogenize the argument list of PTLMC with that of other samplers. My test execution fails due to an array sizing issues I suspect. Moses should first update this test setup to see if it's any good and then check the error.
update feature branch from main
…n addition, to unset an RNG
… any collection-time error
(1) Ensure that no surmise RNG is used except within test functions (2) Reduce redundancy in calling shared variables from shared_scenario.py (3) Allow for fixtures to be called by test to ensure "no_rng" (RNG cleared), "seeded_RNG" (surmise RNG set properly)
As expected, the new results were not identical to the benchmark. However, the new results look reasonable.
Random draws not using the global RNG. Internal tests probably shouldn't override the underlying failure message, but rather only print more information to help understand the context of the failure. This should hopefully get actions passing again.
Implement new RNG scheme
Last time this action ran it needed less than 5 minutes, so 10 should be a nice conservative value that won't waste too much runner time on texlive failures.
Don't let failed texlive installations run forever
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
mainupdated since this branch was based off of it.