# The Flattening Refactor

**URL:** <https://discourse.mc-stan.org/t/the-flattening-refactor/7932>\
**Category:** Developers\
**Created:** [March 5, 2019, 5:18pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932 "2019-03-05T17:18:39Z")\
**Posts on this page:** 20\
**Page:** 1

<div class="post-metadata">

**Author:** ![anon79882417](https://avatars.discourse-cdn.com/v4/letter/a/f05b48/32.png) [@anon79882417](https://discourse.mc-stan.org/u/anon79882417)\
**Post date:** [March 5, 2019, 5:18pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/1 "2019-03-05T17:18:39Z")

</div>

Here’s an update of the flattening refactor, tagging everyone involved or interested:  
@syclik @seantalts @roualdes

Looks like @roualdes’ script worked pretty well. We have all tests passing for the following prim directories: functor, meta, functor and vectorize.

I’m currently tying up some lose ends in prob, err and fun (it’s either some template deduction failures or the includes). After that, with a review, I think we can merge this into develop and start flattening `rev`, `fwd` and `mix`.

All of this is happening on `code-cleanup/issue-937-flatten` if anyone wants to follow along.

---

<div class="post-metadata">

**Author:** ![anon79882417](https://avatars.discourse-cdn.com/v4/letter/a/f05b48/32.png) [@anon79882417](https://discourse.mc-stan.org/u/anon79882417)\
**Post date:** [March 5, 2019, 5:30pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/2 "2019-03-05T17:30:23Z")

</div>

now `prim/prob` is good

---

<div class="post-metadata">

**Author:** ![anon79882417](https://avatars.discourse-cdn.com/v4/letter/a/f05b48/32.png) [@anon79882417](https://discourse.mc-stan.org/u/anon79882417)\
**Post date:** [March 5, 2019, 6:20pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/3 "2019-03-05T18:20:58Z")

</div>

yeah, everything is passing except for the test `cov_matrix_transform_test.cpp`, some templating issue. I’ll figure it out eventually.

---

<div class="post-metadata">

**Author:** ![roualdes](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/roualdes/32/1594_2.png) [@roualdes](https://discourse.mc-stan.org/u/roualdes)\
**Post date:** [March 5, 2019, 6:56pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/4 "2019-03-05T18:56:18Z")

</div>

Great work, thanks.

Let me know if you have some follow up thoughts on how to improve this script for `rev`, `fwd`, and `mix`.

I’m just going to record here some other ideas.

At the end of this effort, go through all the files and remove the excess new lines that my script creates. Single spaced, to me, seems better than what my script currently leaves behind.

I noticed that the following tests are empty save `#include`s. Are these files here to remind us that these functions need tests?

`test/unit/math/prim/mat/fun/typedefs_test.cpp`  
`test/unit/math/prim/mat/err/constraint_tolerances.cpp`  
`test/unit/math/prim/mat/err/validate_non_negative_index_test.cpp`  
`test/unit/math/prim/mat/fun/LDLT_factor_test.cpp`  
`test/unit/math/prim/mat/fun/columns_dot_product_test.cpp`  
`test/unit/math/prim/mat/fun/dot_product_test.cpp`  
`test/unit/math/prim/mat/fun/log_determinant_test.cpp`  
`test/unit/math/prim/mat/fun/mdivide_left_tri_low_test.cpp`  
`test/unit/math/prim/mat/fun/mdivide_right_tri_low_test.cpp`  
`test/unit/math/prim/mat/fun/rows_dot_product_test.cpp`

---

<div class="post-metadata">

**Author:** ![anon79882417](https://avatars.discourse-cdn.com/v4/letter/a/f05b48/32.png) [@anon79882417](https://discourse.mc-stan.org/u/anon79882417)\
**Post date:** [March 5, 2019, 7:35pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/5 "2019-03-05T19:35:38Z")

</div>

> [@roualdes](#):
>
> Are these files here to remind us that these functions need tests?

probably! good catch, we can add this to the list of things to in “paying off math debt”.

> [@roualdes](#):
>
> Let me know if you have some follow up thoughts

I’ll give it some thought after we can successfully merge prim.

> [@roualdes](#):
>
> At the end of this effort, go through all the files and remove the excess new lines that my script creates.

couldn’t agree more.

---

<div class="post-metadata">

**Author:** ![anon79882417](https://avatars.discourse-cdn.com/v4/letter/a/f05b48/32.png) [@anon79882417](https://discourse.mc-stan.org/u/anon79882417)\
**Post date:** [March 8, 2019, 12:39am UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/6 "2019-03-08T00:39:32Z")

</div>

Hey @roualdes -

So with the last commit I just pushed to `code-cleanup/issue-937-flatten`, everything in the refactored prim should compile. I think it’s safe for you to (if you don’t mind) go ahead and remove the extraneous blank lines at the top of the files (or between the includes and the #ifndef statements).

If it’s cool, please make sure the tests compile with your OS (as I’m not running mac OS), and then if that’s all good, go ahead and remove the extra lines.

I’m just getting one test error (it compiles fine) in `prim/fun`, that is, in `test/unit/math/prim/fun/digamma_test.cpp`. Not sure, nothing has changed really. I’ll take a look in the near future.

After both of these items are taken care of, I think we can ping DJ Syclik for a review, and we can get the prim refactor merged.

---

<div class="post-metadata">

**Author:** ![roualdes](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/roualdes/32/1594_2.png) [@roualdes](https://discourse.mc-stan.org/u/roualdes)\
**Post date:** [March 8, 2019, 4:42pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/7 "2019-03-08T16:42:05Z")

</div>

Do you have an example file where blank lines happen

> [@anon79882417](#):
>
> between the includes and the #ifndef statements

? I know there’s plenty of consecutive blank lines, that I plan on removing, but I can’t immediately see why my `flatten.sh` script would introduce blank lines between directives.

Either way, I’ll get started this weekend.

---

<div class="post-metadata">

**Author:** ![anon79882417](https://avatars.discourse-cdn.com/v4/letter/a/f05b48/32.png) [@anon79882417](https://discourse.mc-stan.org/u/anon79882417)\
**Post date:** [March 11, 2019, 11:37am UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/8 "2019-03-11T11:37:41Z")

</div>

Sorry for the delay.

There is extra white space between `#include` statements and `namespace stan {` statements.

Also, in for example, `stan/math/prim/fun/common_type.hpp`, we have two `namespace stan` declarations, with many white lines in between. If it’s easy to handle this via bash scripting, I would go ahead and fix it, but if not, I think we’re fine as long as all tests compile and pass.

But pretty much every file has extra blank lines that will cause lint errors and should be removed.

Andre

---

<div class="post-metadata">

**Author:** ![sakrejda](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/sakrejda/32/845_2.png) [@sakrejda](https://discourse.mc-stan.org/u/sakrejda)\
**Post date:** [March 11, 2019, 12:56pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/9 "2019-03-11T12:56:38Z")

</div>

You might want to check that the formatter doesn’t just take care of this

---

<div class="post-metadata">

**Author:** ![syclik](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/syclik/32/6_2.png) [@syclik](https://discourse.mc-stan.org/u/syclik)\
**Post date:** [March 11, 2019, 3:48pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/10 "2019-03-11T15:48:04Z")

</div>

@anon79882417 and @roualdes, current status? And what’s the rough plan? (How many PRs to get to the finish?)

---

<div class="post-metadata">

**Author:** ![roualdes](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/roualdes/32/1594_2.png) [@roualdes](https://discourse.mc-stan.org/u/roualdes)\
**Post date:** [March 11, 2019, 4:15pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/11 "2019-03-11T16:15:14Z")

</div>

> [@anon79882417](#):
>
> Sorry for the delay.

No worries, the way my weekend went I’m bound to be the slowest link here.

> [@anon79882417](#):
>
> two `namespace stan` declarations

Right, I did not pay attention to multiple `namespace` declarations because tests will still pass despite this. I’m hoping the same strategy for cleaning up multiple `#ifndef`s should work here. I’ll try to remove these along with double blank lines.

> [@syclik](#):
>
> And what’s the rough plan?

I’m about to start tidying up `prim` today. Then we’ll submit a PR for a backwards compatible, flattened `prim`.

It’s my understanding that we’ll have one PR for each of `prim`, `rev`, `fwd`, and `mix`. So 4 total.

> [@sakrejda](#):
>
> formatter

Thanks, I’ll look into that.

---

<div class="post-metadata">

**Author:** ![anon79882417](https://avatars.discourse-cdn.com/v4/letter/a/f05b48/32.png) [@anon79882417](https://discourse.mc-stan.org/u/anon79882417)\
**Post date:** [March 11, 2019, 7:52pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/12 "2019-03-11T19:52:22Z")

</div>

@roualdes regarding @sakrejda’s comment, I think the autoformatting will actually handle on the formatting issues you were going to address.

Let me just figure out why this one test isn’t passing, open a PR, and I think the autoformatter will handle it all.

I’ll ping you and let you know if we actually need to format this stuff manually.

Shooting to have this one test case figured out today.

---

<div class="post-metadata">

**Author:** ![roualdes](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/roualdes/32/1594_2.png) [@roualdes](https://discourse.mc-stan.org/u/roualdes)\
**Post date:** [March 12, 2019, 12:31am UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/13 "2019-03-12T00:31:21Z")

</div>

@anon79882417, from what I can see, the auto-formatter handles the blank spaces, but not the multiple `namespace`s.

I’m fairly close to a solution, for files that have commented the closing braces of their namespaces, e.g.

```
namespace stan {
namespace math {
// ...
} // namespace math
} // namespace stan

```

This could be a cleanup issue, if we want.

---

<div class="post-metadata">

**Author:** ![anon79882417](https://avatars.discourse-cdn.com/v4/letter/a/f05b48/32.png) [@anon79882417](https://discourse.mc-stan.org/u/anon79882417)\
**Post date:** [March 12, 2019, 5:56pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/14 "2019-03-12T17:56:05Z")

</div>

As long as the code compiles and tests pass, I don’t see a need to add it to the current flattening, as it’s purely aesthetic, and serves no functional purpose. Unless @syclik really thinks this is an issue, then we can set a separate issue about it. It’s more important to get the flattening working first.

---

<div class="post-metadata">

**Author:** ![Bob\_Carpenter](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/bob_carpenter/32/9230_2.png) [@Bob\_Carpenter](https://discourse.mc-stan.org/u/Bob_Carpenter)\
**Post date:** [March 12, 2019, 6:08pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/15 "2019-03-12T18:08:16Z")

</div>

> [@anon79882417](#):
>
> Looks like @roualdes’ script worked pretty well. We have all tests passing for the following prim directories: functor, meta, functor and vectorize.

This is super cool! I’m so looking forward to this.

Wherever possible, tests can be moved to use higher-level includes like `rev/mat.hpp`, etc.

---

<div class="post-metadata">

**Author:** ![syclik](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/syclik/32/6_2.png) [@syclik](https://discourse.mc-stan.org/u/syclik)\
**Post date:** [April 12, 2019, 2:12pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/16 "2019-04-12T14:12:27Z")

</div>

@anon79882417, @roualdes: how’s it going? Mind posting up a summary? Is there some way I can provide support?

I still haven’t seen a plan for how to go from the current state to the end state. Do you have one and I’ve missed it?

---

<div class="post-metadata">

**Author:** ![roualdes](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/roualdes/32/1594_2.png) [@roualdes](https://discourse.mc-stan.org/u/roualdes)\
**Post date:** [April 12, 2019, 4:24pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/17 "2019-04-12T16:24:56Z")

</div>

@syclik, thanks for checking in. We’re making progress, but have a number of compilation errors remaining.

We continue to bounce back and forth between me and @anon79882417. I add a little bit to the bash script we’ve been calling `flatten`; found as a gist [here](https://gist.github.com/roualdes/ea72fc0fbcac05329e562a0e60ee0a1d). Andre solves many of the compilation errors based off my last run of `flatten`, and then I add those changes to the script.

The latest bit we’ve focused on is adding Bob’s suggestion for the tests to use higher-level includes.

It’s my understanding that our plan is to continue developing `flatten`, flattening all directories at once, until we are satisfied with our automated solution. After this, we’ll ask for people to delay merging into develop, merge develop into our branch, run `flatten`, and ask for a review.

---

<div class="post-metadata">

**Author:** ![anon79882417](https://avatars.discourse-cdn.com/v4/letter/a/f05b48/32.png) [@anon79882417](https://discourse.mc-stan.org/u/anon79882417)\
**Post date:** [April 17, 2019, 11:46pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/18 "2019-04-17T23:46:21Z")

</div>

Thanks, @roualdes.

We have all tests in prim compiling and passing, and I’ll start working on `rev`, `mix` and `fwd` soon.

---

<div class="post-metadata">

**Author:** ![syclik](https://yyz2.discourse-cdn.com/flex030/user_avatar/discourse.mc-stan.org/syclik/32/6_2.png) [@syclik](https://discourse.mc-stan.org/u/syclik)\
**Post date:** [April 18, 2019, 2:08pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/19 "2019-04-18T14:08:41Z")

</div>

> [@roualdes](#):
>
> It’s my understanding that our plan is to continue developing `flatten` , flattening all directories at once, until we are satisfied with our automated solution. After this, we’ll ask for people to delay merging into develop, merge develop into our branch, run `flatten` , and ask for a review.

Mind getting into more detail here? I think it’d be much better to split this out into multiple steps, but I can see how it’s done at once.

Asking for everything to stop on the Math library is a hard ask. It’s not impossible, but it is a bit inconvenient, especially if the Math library has to be locked down for an unspecified amount of time while this is in review.

If you want chat to plan this out, just let me know. For example, I think it’d make a lot of sense if this was done in small stages like:

1. Add the top-level `meta` folders: `stan/math/{prim, rev, fwd}/meta/` with files that are ~5 lines: header guards and an include for the `mat` version of the file. Update source and test files that use `stan/math/*/*/meta/` with the new file. (This leaves all the source intact and doesn’t require any synching across branches.) Maybe move the tests here or add another step to just move all the tests.
2. Combine the `meta` headers into the one in `stan/math/{prim, rev, fwd}/meta/`. Remove the `stan/math/{prim, rev, fwd}/*/meta/` folders. (This leaves most of the Math library intact and everyone else that’s not working on `meta` should be ok… the merge conflict here is easy to deal with, but most likely, there won’t be a merge conflict and git will handle it automatically.)

Then working through each of the folders separately.

I’m really, really weary of changing the whole code base at once. It’s too hard to review and too hard to verify that things don’t break.

Let me know what you think… I’m not trying to say that’s the way you need to do it or that it’s the only way. That’s just one way to do it that’s minimally disruptive to the Math library.

---

<div class="post-metadata">

**Author:** ![anon79882417](https://avatars.discourse-cdn.com/v4/letter/a/f05b48/32.png) [@anon79882417](https://discourse.mc-stan.org/u/anon79882417)\
**Post date:** [April 18, 2019, 2:22pm UTC](https://discourse.mc-stan.org/t/the-flattening-refactor/7932/20 "2019-04-18T14:22:36Z")

</div>

> [@syclik](#):
>
> Asking for everything to stop on the Math library is a hard ask

We don’t have to stop everything, I can just go clean up the stuff in the middle that wasn’t refactored with this PR. Once the flattening gets reviewed, we can go ahead and ask contributors to use the new structure. Not many people will be affected this way.

> [@syclik](#):
>
> I’m really, really weary of c

It’s just difficult to do one directory at a time, we’d be modifying other directories anyway to get tests to pass. We can look into it again

[Next page](https://discourse.mc-stan.org/t/the-flattening-refactor/7932.md?page=2)