The Flattening Refactor

Developers
41

Metaprograms are properly flattened for prim and the same script should work for fwd, rev and mix. Here’s a hacked script, that’s much uglier than Ed’s: flatten_prim_meta.sh

All that’s left to do before I open the prim PR is write tests for the rest of meta (I think there are 4).

There’s also files that weren’t moved by Ed’s script for whatever reason (probably about 20, I have a list compiled), that I’ll either write a bash script to flatten them or do it manually, whatever’s faster.

~~

I’m very, very afraid of destroying Stan. After I get prim working I’m going to do Stan-dev/Stan’s compile model tests to make sure no compilation breaks.

Can anyone with more experience suggest anything else to test at a higher level before the PR goes through? The math tests can compile and the language @ all devs who touch stan-dev/math and stan-dev/stan.

Should I do any testing for the stanc3 ocaml compiler? @seantalts

1 Like
42

Nope, don’t worry about the new compiler :)

I’d suggest running these performance and numerical gold tests https://github.com/stan-dev/performance-tests-cmdstan/blob/master/README.md#testing-one-git-commit-against-another They are probably the best real-world coverage we have, though they take forever and coverage is far from 100%.

1 Like
43

This won’t be done this week. I’m still weeding through 100’s (or more) of compiler errors after “flattening” prim. They can take anywhere from a few seconds to hours for me to figure out. I’m just going through one at a time, fixing it in one case, and then writing a script that fixes it everywhere else in the library.

44

I’ve been ignoring this because it was frustrating. I’m introducing more “tangles” by trying to do prim separately. I was trying to be nice and easy to work with, but this has the unfortunate consequence of wasting time.

I’m going back to using Roualdes’ script (because stuff was actually compiling and tests passing when we did everything at once), and then I’ll do the Seam for meta after.

Sorry this is taking forever.

45

@syclik mind just creating a separate issue for the meta Seam? We can just handle that in another PR.

46

yeah, everything in prim compiles with this.

If anyone has time and wants to take a look, for whatever reason ./runTests.py test/unit/math/rev/vectorize/apply_scalar_unary.hpp isn’t finding Eigen::Internal. I’ve tried explicitly including some stuff directly from Eigen, but no luck. I’ll come back to it. Probably something like re-ordering includes may help.

The branch is code-cleanup/issue-937-flatten.

47

I updated my copy of this branch, but the following doesn’t match any tests.

48

I don’t think anyone thought this would be easy.

Is there a complication beyond not finding includes?

49

@syclik anything from a quick inspection wrong with these files?

50

I think the first, apply_scalar_unary.hpp, shouldn’t include <Eigen/Dense> directly – it should be including our headers.

The third, acosh.hpp, that shouldn’t be including prim.hpp. It should only include the headers for these things: check_greater_or_equal, apply_scalar_unary, and <cmath>.

2 Likes
51

yeah, if you want to take a look at that branch, at stan/math/rev.hpp and there’s obvious mistakes, that could potentially save me hours of time.

52

@anon79882417, I put up one PR: https://github.com/stan-dev/math/pull/1259

It separates out meta from everything else. This will make life easier and hopefully it’ll save you from seeing some of those compiler errors.

Maybe we can separate the refactor so I’m working on the meta traits and you and @roualdes can flatten everything else?

53

OK~

1268 Was merged.

now we only include prim/meta.hpp at the tops of our files if we need any of the meta programs.

(or rev/meta.hpp, or mix/meta.hpp or fwd/meta.hpp depending on what you’re working on)

3 Likes
54

Nice!

55

That’s huge! Thank you so much @anon79882417, @roualdes, and @syclik. I’m so excited - this is going to make development and developer onboarding so much simpler going forward. I owe you all a tasty beverage!

56

Yeah, tomorrow/this weekend I’m looking at editing @roualdes script to include some missing ifdefs then avoid the meta areas, and only act on prim. If we’re lucky, after some manual editing were good to go. But most likely not that lucky

57

@anon79882417, you’re on a roll! Is there anything I can do to help?

58

No, thanks. I’m just editing your script. It would just be me asking you to write lines of code that I can write myself

59

@anon79882417 I merged PR #1234 that cleans up the metaprograms a bit. I will pause my meta PRs that I have been doing under issue #1122 until the meta flattening refactor happens. This last one was open before the flattening refactor started and I wanted to close it before it happens.

60

Thanks.

I’m giving it another shot today. Yesterday I tried naively combining some of the meta files, but when I remove the mat/scal or arr from the pathway of a single include in meta.hpp, there are “over includes” (some of the same files being included more than once) and the compiler flips.

So today, I’m aiming to reduce dependencies by doing some of the following, in no particular order: 1. Remove meta.hpp includes from mat.hpp, scal.hpp, arr.hpp; 2. only include arr.hpp in all files (so we’re not over including, and this should be fine because arr.hpp includes mat.hpp includes scal.hpp), etc, etc… things like that. just generally try to untangle dependency structure.

1 Like