Open syclik opened 9 years ago
This seems like a major issue. @rtrangucci --- it sounds like you figured out how to fix this --- would you mind putting the fix in the form of a pull request? Or saying a bit more about what you mean by "change order". I'm happy to do the mechanical bits, but that's an incredibly complicated fixture. I'm upping the due date to the next release.
From @rtrangucci on January 21, 2015 1:15
the test_gradients test in test_fixture_distr.hpp, test_fixture_cdf.hpp, test_fixture_cdf_log.hpp and test_fixture_ccdf_log.hpp is broken.
The test_gradients in test_fixture_distr.hpp is intended to test a density's implementation of partials with an autodiffed naive implementation of the density. However, due to the current pattern, the test always passes. To verify this, change line 87 in stan/src/test/unit-distribution/univariate/continuous/normal/normal_test.hpp from:
return -0.5 * (y - mu) * (y - mu) / (sigma * sigma)
to:
return -0.5 * (y - mu) * (y - mu) / (sigma * sigma) * 100
and run:
./runTests.py -j2 src/test/unit-distribution/univariate/continuous/normal/normal_00000_generated_v_test.cpp
noting that the "Function" tests for v32 and v36 pass when they shouldn't. The problematic section in test_fixture_distr.hpp are lines 522-552:
The reason the tests pass is because calculate_gradients_1storder in stan/src/test/unit-distribution/test_fixture_distr.hpp calls
stan::agrad::recover_memory()
and the naive implementation and the density function are called with the same vars. y1 and x1 point to the same varis, whose adjoints are updated when calculate_gradients_1storder is called the first time.The fix is to change the order of how calculate_gradients is called and how vars are created for the naive implementation vs. the stan::prob densities within test_gradients.
Copied from original issue: stan-dev/stan#1224