search

stan-dev / stanc3

The Stan transpiler (from Stan to C++ and beyond).
BSD 3-Clause "New" or "Revised" License
140 stars 44 forks source link

Disable AD levels optimization in `--O1` #1149

Closed rok-cesnovar closed 2 years ago

SteveBronder commented 2 years ago

Thanks Rok! I thought we tested all of these with O1 before but I must have missed something. I will go through these today to see whatsup. My initial guess is that turning off the partial evaluator will fix a lot of these

SteveBronder commented 2 years ago

fyi running a jenkins job with a math PR to check if I have the math fixes right

https://jenkins.flatironinstitute.org/blue/organizations/jenkins/Stan%2FStanc3/detail/PR-1149/7/pipeline/249

SteveBronder commented 2 years ago

Okay! Now I want to turn on the partial evaluator to see if my math fixes also fix that

SteveBronder commented 2 years ago

Mkay! So it looks like with those two math PRs we are good for the partial evaluator

rok-cesnovar commented 2 years ago

@SteveBronder does that mean that we merge the 2 math PRs and we should be good to go for 2.29.2?

rok-cesnovar commented 2 years ago

Oh and disable AD levels for O1?

SteveBronder commented 2 years ago

Yep!

WardBrian commented 2 years ago

Is the AD levels optimization a low impact one?

SteveBronder commented 2 years ago

Its an optimization where imo the user could also just move the data only operation to transform data themselves. imo not a huge one. I think part of the issue is just that we shouldn't run it over functions. The main place it is beneficial is in log_prob

rok-cesnovar commented 2 years ago

Both Math PRs were just merged in, I cleaned up this PR which will now just disable AD levels optimizations in O1. Running it once more with --O1 to make sure all models compile and will then remove changes to Jenkinsfile and tag for review.

rok-cesnovar commented 2 years ago

Agreed! See https://github.com/stan-dev/performance-tests-cmdstan/pull/37

Once that is in we just need to add a Jenkins stage to run it.