-
-
Notifications
You must be signed in to change notification settings - Fork 190
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
Fold grad_inc_beta into grad_reg_inc_beta #3159
Conversation
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
Test failure looks like it's just something being flakey up in cmdstan? |
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
Huh, it seems to be repeatable here. @andrjohns are you on a Mac to try plugging this branch into cmdstan? |
Absolutely bizarre! Yeah I'll do some testing locally and see what gremlin I've unleashed in the code |
I can't reproduce locally unfortunately, and the error on Jenkins seems strange. It's failing because it can't find some of the object files during linking:
And I can see earlier in the log that the object files are getting cleaned up in a different step before the linking call. Not entirely sure how/why it's happening there tbh |
314398e
to
96b8a66
Compare
Just rebased onto |
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
It's weird, because other PRs haven't seen the same failures, so it does seem like this change would be responsible, but I can't imagine why |
@andrjohns independent of the failures, it looks like there are a few comments in the grad_2F1 file that should be updated (they specifically mention |
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
I manually ran https://jenkins.flatironinstitute.org/blue/organizations/jenkins/Stan%2FCmdStan/detail/downstream_tests/829/pipeline/6 with this branch but using We might be able to use |
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
I cannot fathom why this is still happening... and it's only for this PR, |
Hmmm, could a workaround for now be to run the tests with |
That would probably fix it, but I'm still suspicious there might be an actual issue here we just don't understand. If you look at the history for the downstream branch, the recent failures are exclusively for this PR, and there are plenty of other successes |
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
Okay, I think it is probably just a race condition on the disk. For some reason, the cmdstan runTests python script does batching, and I think that these batches end up running with some small overlap in time to each other, so the next batch sees that the previous batch built the Increasing the batch size large enough so that there is only one batch both seem to fix it. The entire runCmdStanTests.py script is pretty ugly, so I'm not too miffed at just bumping the BATCH to like 50 (from 25) and moving on... |
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
Jenkins Console Log Machine informationNo LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 20.04.3 LTS Release: 20.04 Codename: focalCPU: G++: Clang: |
Summary
The
grad_inc_beta
function is currently only used in thegrad_reg_inc_beta
function. The two functions share similar variables, leading to redundant computation.This PR folds the computations for
grad_inc_beta
into the existinggrad_reg_inc_beta
function.Tests
N/A - Existing tests should still pass (no existing tests for
grad_inc_beta
)Side Effects
N/A
Release notes
Removed redundant computation in
grad_reg_inc_beta
Checklist
Copyright holder: Andrew Johnson
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit
)make test-headers
)make test-math-dependencies
)make doxygen
)make cpplint
)the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested