Conversation
|
@SteveBronder mind having another look? I made a few small changes and now all of our tested models compile with --Oexperimental |
SteveBronder
left a comment
SteveBronder
left a comment
There was a problem hiding this comment.
You do a lot of overwriting of names here that would be easier to follow if the code did not do that. i.e. in TernaryIf (e1, e2, e3) there is a lot of overwriting of e1, e2, and e3
Besides that one small thing and I think it looks good
| let e2, e3 = | ||
| match all_or_nothing [e2; e3] with | ||
| | [e2; e3] -> (e2, e3) | ||
| | _ -> assert false in |
There was a problem hiding this comment.
Why the assert false here? Should we have a more meaningful error message?
There was a problem hiding this comment.
The function is essentially a map, so it’s impossible for the value to change length compared to the input
| (* hack: optimize_ad_levels should only need to run after optimize_soa, | ||
| but because it can turn variables into data (which are all AoS) it can | ||
| lead to compilation errors due to missing overloads in math. So we run | ||
| it twice to prevent variables that will interact with demoted variables | ||
| from ever getting tagged as SoA compatible. *) |
There was a problem hiding this comment.
Steve I wrote that comment all by myself
|
I like name shadowing for these purposes because it prevents you from making the easy mistake of using the old value |
2 participants
This is an improvement on #1713 and #1715 and fixes the
--Oexperimentalcompilability of several models:And possibly more, I'm running additional builds now.
The main change is to be stricter about how we assign autodiff types to tuples, and to improve the handling of promotions (especially inside of containers) in the optimize_ad_levels pass.
Submission Checklist
Release notes
Fixed several more issues with
--Oexperimentalgenerating invalid C++Copyright and Licensing
By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)