Drake: convert textbook to use c++ examples

[RussTedrake]

I am working on converting the textbook / drake tutorial to use the new c++ classes. To make it work well, I think I need to change the relationship between the textbook and the drake repo.

Proposal:

• textbook moves out of drake/doc and into externals/textbook
• textbook compiles after drake, and contains it's own src/ directory for the examples which are still compiled/run as part of drake's CI.
• the simplest examples (like simple_continuous_time_system.cc) move out of drake/examples and into textbook/src

My questions/observations/concerns:

• this is very similar to the proposed model for director (build as an external after drake)
• changes to drake that would break the textbook examples are a potential pain point (presumably for director, too). will require PR'ing to the textbook, and updating the SHA. I can try to keep these dependencies very minimal to begin with.
• textbook examples won't benefit from the same code review without me asking (but I do intend to ask). they also need not have the same linters/etc enforced, and will not appear in the doxygen.

Otherwise, I actually think this is a cleaner (more explicit) relationship between the two. Soliciting feedback now in case there are objections, or if people foresee an issue that I've missed.

@jamiesnape , @david-german-tri, @jwnimmer-tri -- would value your feedback in particular.

[liangfok]

I'm not sure I understand the benefits of moving the textbook's examples out of drake/examples. Why not keep the examples in drake/examples while moving the textbook's text into a different repository? That would ensure the examples keep pace with the latest version of Drake while allowing the textbook's text to be updated without going through Drake's review process. The textbook's text can still refer to the examples in drake/examples when it is in a separate repository.

Maybe a README.md can be included with each textbook example stating "this example is referenced by the textbook" to prevent it from being accidentally removed.

[RussTedrake]

Good. Thought a bit about this before i recommended this path.

The main problem is versioning. we don't want developers in drake to move/modify/remove some files that are changing lines in the textbook without knowing it and/or reading the surrounding text in the book.

There might also be times where i violate the style guide in small ways for the sake of clarity/readability. For instance keeping templated method implementations in the header file so i can show an entire class implementation at once.

It happens there was also a technical motivation -- it's harder to accomplish a server side include with ajax from a neighboring directory than from a sub directory. I'm sure that is surmountable.

[liangfok]

I see, makes sense. I'm now wondering whether two copies of the textbook examples should be maintained, one in drake-distro/drake/examples and another in the textbook's repository. The textbook will be pegged to a particular version of Drake and only occasionally updated to the latest on master. When it is updated, the updating process will be easier because the examples in drake-distro/drake/examples would have kept pace with the latest on master meaning the changes necessary to update the textbook can be obtained using git diff.

I would not mind adopting a policy where clarity-enhancing style guide violations are OK for textbook examples.

[RussTedrake]

The duplicated code in the textbook repo is what we have currently (in
matlab). I'm potentially ok with that, too. But i also have heard people
questioning whether any examples really belong in the main repo, and this
could be an opportunity to purge a few of the simplest ones.
On Sun, Oct 16, 2016 at 8:21 AM Chien-Liang Fok [email protected]
wrote:

I see, makes sense. I'm now wondering whether two copies of the textbook
examples should be maintained, one in drake-distro/drake/examples and
another in the textbook's repository. The textbook will be pegged to a
particular version of Drake and only occasionally updated to the latest on
master. When it is updated, the updating process will be easier because the
examples in drake-distro/drake/examples would have kept pace with the
latest on master meaning the changes necessary to update the textbook can
be obtained using git diff.

I would not mind adopting a policy where clarity-enhancing style guide
violations are OK for textbook examples.

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
https://github.com/RobotLocomotion/drake/issues/3815#issuecomment-254043849,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGJNNN1zgG5Gl5IUk8gEMtXwSD86OOJXks5q0haygaJpZM4KX3tt
.

[liangfok]

👍 for eventually removing all examples from the main repo. As the main repo's code gets increasingly covered by unit tests, the testing value of the examples diminishes meaning the need for them to be in the main repo also decreases.

As we increase the number of repositories related to Drake, it seems what we need is a coordination layer that can atomically merge multiple PRs submitted to mutiple repositories and update multiple submodule SHAs spread across multiple repositories. I wonder if such a layer already exists for a github-based workflow.

[david-german-tri]

Repositories are good for controlling ACLs, merge policy, and the minimum unit of download. If it's necessary to de-ACL drake developers from the textbook, then I think the original post is a reasonable way to go. However, I also think it would be more convenient just to isolate the textbook and its examples in a clearly-labeled subdirectory. Then we wouldn't have to juggle version numbers, or add post-Drake superbuild steps for CI purposes.

Aside @liangfok: I don't agree that /examples should eventually leave Drake. They have the same ownership as the rest of Drake, and download size and scope are not issues, so why jump through extra hoops to maintain them?

[liangfok]

@david-german-tri, I think removing examples from Drake would fall in the "minimum unit of download" category. I agree, however, that we should not remove them if we don't have the tooling to keep them updated with the latest on master.

[jwnimmer-tri]

I don't think the goal is to de-ACL folks. If we change the System APIs, we'll need to update the textbook to pass also (in coordination with Russ). Similarly for merge policy -- if we're sharing CI, then the merge policy is the same (or at least very, very similar). We can certainly put a drake-distro/drake/textbook/CPPLINT.cfg if we want to nerf certain warnings, for example, but still have the code in-tree. Similarly, minimum unit of download doesn't seem like an issue.

In short: @RussTedrake What problem are you trying to solve here? You clearly state a proposed solution, but I am unable to back-figure the problem.

[RussTedrake]

As the subject says -- i'm trying to convert the textbook to c++. it needs to have some example code in it. I would like that code to be tested so that it compiles and runs with drake. therefore the textbook either needs to reference directly into drake master (#include drake/examples/blah) or have its own src directory which runs the tests.

[jwnimmer-tri]

I think that David and I are both firmly in the camp of "the more repos we have, the less happy we are", and thus are implicitly proposing that as much code or docs live in-tree with Drake as possible.

[david-german-tri]

Specifically, I was suggesting that drake/examples/textbook or similar might be good enough to protect the textbook example code from accidental changes (the stated primary goal), without the overhead of keeping repos in sync.

As for style, I think bending the rules in an external that we cover with CI is equivalent to bending them in a special directory on master. Either way, developers who break the tests have to sift through out-of-style code to debug. (Of course, if the examples are simple enough, this may be a cost worth bearing.)

[jamiesnape]

I guess it partly comes down to API stability. If the API is changing a lot and breaking examples, it would be more convenient to be able to update the API and fix the examples together in one PR. It also makes the change logs in Jenkins and CDash more readable.

There is a certain advantage to having the examples above the drake directory as you get to test things like find_package(drake) if and when that exists.

On the other hand, if examples are in drake/examples/textbook, CI needs no changes at all and would work immediately.

[RussTedrake]

it's worth noting that the current state is that we have the textbook in a separate repo, owning its own example code, which could break on an API change to drake. that has not happened yet because it was only matlab example code. the fact that it gets loaded into drake/doc/textbook avoids the need for find_package(drake), but is otherwise almost irrelevant.

i do think that the textbook text belongs in a different repo -- it is logically separable and I will need to commit text relatively often that the drake developers probably don't care to review. it's also convenient to be able to grab the textbook without all of drake.

[jamiesnape]

Seems reasonable to me. Having the submodule reference in drake/examples/textbook or the current location may be more convenient for now as we do not quite have everything that find_package needs yet.

[jwnimmer-tri]

The situation I want to avoid is having to frequently coordinate PRs between Drake's APIs and the textbook. Once every ~2 months would be okay; more often would be a drag. That is, if Drake devs can essentially pretend its not there, then it doesn't matter how the build is structured. However, if we have to pay attention and fix it, then doing so should be low friction, e.g., either (1) the code we need to fix is in-tree with Drake, or (2) the textbook uses a tag revision, which we bump with regularly-scheduled ceremony.

[jamiesnape]

(2) the textbook uses a tag revision

This would necessitate a separate CI for textbook.

(Not necessarily a problem, but certainly a difference.)

[RussTedrake]

this has been working great (and I've survived the term)