9

I write code in R, and often find myself attempting to optimize the code for better performance. In a given script that tackles a specific problem, I test different code alternatives and compare them to each other with benchmarking. At the end, I select the most performant method. However, I don't know how to document those benchmark tests.

I'll use an example to demonstrate (based on a real problem I asked about). In R, I want to write code that nests a dataframe by group. I have three possible methods I compare:

bench::mark(dplyr = mpg %>% group_by(manufacturer, year) %>% summarise(nest_cty = list(cty)),
            data.table = {MPG <- data.table(mpg); MPG[, .(nest_cty = list(cty)), by = list(manufacturer, year)] },
            collapse = mpg %>% fgroup_by(manufacturer, year) %>% fsummarise(nest_cty = list(cty)),
            check = FALSE)
#> # A tibble: 3 x 6
#>   expression      min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr> <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 dplyr        4.69ms    5.5ms      184.    2.38MB     5.56
#> 2 data.table   2.37ms   2.51ms      391.    2.16MB     0   
#> 3 collapse     95.2us  101.8us     9560.  206.56KB     6.22

The benchmarking table reveals that the third option is the most performant. So when finalizing my script, I'll choose that method. But I still want to document that I've tested different methods, and the results of the benchmarking, so future-me or collaborators could understand my choices.

How should I document this? I understand that writing such "story" in comments, inside the script, is considered a bad practice. Other option is via git commits. However, I find it too verbose to include such explanation in the description of a git commit. Furthermore, git commits have to do with tracking changes in code, but my need here is more of a metainformation about general strategy rather than specific change in code.

8
  • 4
    Does your code have actual english-language (or other-language) documentation? A section on 'theory' or 'overview' wouldn't be too unusual.
    – 2e0byo
    Commented Nov 5, 2021 at 21:26
  • @2e0byo, what I gather from your comment (correct me if you meant otherwise) is an overarching documentation about the code on the project-level. But my question is aimed at very high resolution, focusing on certain lines of codes: e.g., "the method used in lines L25-L32 is x rather than y due to such and such benchmarking".
    – Emman
    Commented Nov 6, 2021 at 6:26
  • 1
    Emman I meant something very like what @Christophe's answer describes, although I didn't actually know the phrase 'design document'. In any case the answers and comments cover the issues much more thoroughly than I likely could :) Before working on an opensource project, though, I really do look for a 'design' or 'implementation' or 'contribuing' or 'considerations' section and read it if it's there. Nobody has really mentioned the commit suggestions, but I (one data point!) very rarely run git blame before doing something; maybe I should. This might not be relevant to your field/usecase.
    – 2e0byo
    Commented Nov 6, 2021 at 17:13
  • You can create an alternate git branch for it. This way it is only there when you want it to be. In the end, it is up to you. There is no hard and fast rule for the process. Just ensure that, if you need it for whatever reason, it is easily obtainable. If you don't need it, it is a distraction from what you do need; hide it.
    – Nate T
    Commented Nov 7, 2021 at 0:08
  • 1
    Where I work, we use branches extensively, but we also have well documented standards that govern the our of branches. For this reason, in my case it makes sense to use them. If you are a part of a larger team working on this project and you are not a tech lead or project manager, then it is not your place to decide how to document this. It is the responsibility of the team coordinator to decide ensure that everyone is on the same page. The method doesn't matter, so long as it makes sense and everyone knows it. This is what standards are for: to ensure that everyone is on the same page.
    – Nate T
    Commented Nov 7, 2021 at 19:53

4 Answers 4

18

It is not bad practice to put this as a comment in code. Your goal is not to show everyone you did your homework, you want to prevent any successor to go "this is silly, I can do this more elegantly" => typer-de-type, fixed!. The only place to make this work is in the code. You can be brief, no tables with test results, just "it turned out this was about x times faster than that so I went with this solution instead of that one". Putting this in a design document is pointless, nobody will read that at the time it matters.

8
  • I agree that a short sharp sentence "used x because ten times faster than y" would be an ideal comment. However, if this access strategy is used in 20 different places, would you repeat this comment 20 times just for not risking the point being missed? Anyway, the sharp sentence seems not sufficient for summarizing a benchmark and explaining the rationale: aren't comments supposed to document the current state of the code? Wouldn't historical reasons and pros and cons, benchmarks etc... clutter the code with unnecessary lengthy wording and make browsing the code unpractical?
    – Christophe
    Commented Nov 6, 2021 at 11:18
  • 4
    @Christophe - if the "access strategy" or whatever it is is used in 20 places then you need to use procedural abstraction to put it in one place. And then, that's where you put your documentation. Once. (And then you can give it an appropriate name, too.)
    – davidbak
    Commented Nov 6, 2021 at 13:23
  • 3
    Also, you can embrace the power of "and". As in: put it in a short document (a one page note) that just describes this particular design choice (with its benchmarks) and leave a comment that says "performs better than alternatives attempted, see access-foo-benchmarks.md". Don't lose the information - someone else may discover that there's yet another alternative you haven't thought of, or a later version of the software has changed the behavior sufficiently that something you rejected now works better. But in the code: just a brief comment with the pointer to the doc.
    – davidbak
    Commented Nov 6, 2021 at 13:28
  • 1
    @davidbak an access strategy is a general principle and it may be too general for a procedural abstraction. The trick with the access-foo-benchmark.md is indeed a great idea for explanations that are not important enough for being in a design document and too lengthy to be in the comments (my point here is: too lengthy comments shall be avoided). Alternatively, if benchmarks are used to back a general technique that you may reuse across projects, you may even consider a comment that refers to bigtable-grouping-pattern.md. After a while OP could publish a book on "Effective R patterns" ;-)
    – Christophe
    Commented Nov 6, 2021 at 13:41
  • 1
    We can all get along. How about a URL in a comment that links to a page in an online design document? Peace.✌ Commented Nov 6, 2021 at 14:49
6

This is the kind of information that you could document in some design document. The purpose is to keep track of important choices that were made and why.

Choices with alternatives that were seriously considered would be good candidates in such documents, especially if it's a key element in your algorithm or a general strategy/technique/pattern that you may use in several places (as your real problem suggest). This avoids loosing time reassessing over and over again the same questions.

Comments in the code should remain concise and sharp and should not distract with lengthy justification and historical reasons.

Edit: Strategies/techniques/patterns that are too specific for the general design, but worth to be known and reusable, could be explained in a separate design pattern document with all the justifications needed. As suggested by @davidbak in the comments, you may refer to this document in a concise comment, having the advantages of both approaches.

7
  • 3
    I would regard this kind of choice an implementation detail. A design document should not become a "the making of" documentary. Commented Nov 6, 2021 at 8:11
  • @MartinMaat Thank you for the opportunity to clarify. From what I read in the question, and not being myself R-literate, I am not able to judge if this is implementation detail or a key element in the algorithm. Moreover, I don't know if it's a general choice (a strategy that may be reapplied in many different places) or a purely opportunistic single-use one (I hope OP does not benchmark every single statement). This is why I clarified immediately in the second sentence "keep track of important choices" (and not "every choices like in a making-off").
    – Christophe
    Commented Nov 6, 2021 at 11:06
  • 2
    @MartinMaat - There are design documents of various kinds and various scopes. This kind isn't the describe-the-system-detailed-design-with-section-headings-and-formal-signoffs kind. This kind is the one-pager with focus on one particular design decision. And the OP's issue isn't an implementation detail if it affects overall performance sufficiently that he had to (i.e., felt it worthwhile to) discover and benchmark various alternatives. It is worth a design note so successors can know what was attempted (so they can try something else, or so they can see if things have changed).
    – davidbak
    Commented Nov 6, 2021 at 13:33
  • @davidbak I edited the answer to add the result of our exchanges across the different answers. I think it's an interesting alternative worth to be considered, for elements that are not important enough for being in the design document: it would allow to keep the comments clean and at the same time keep relevant knowledge that might otherwise get lost.
    – Christophe
    Commented Nov 6, 2021 at 14:01
  • @MartinMaat I agree with David: there's perhaps a misunderstanding about the kind of design documents that I mean. I meant concise long term assets, of key decisions (that are not so quickly obsolete btw) and that really help new team members.
    – Christophe
    Commented Nov 6, 2021 at 14:13
2

I would suggest a different approach. You did work benchmarking collecting numbers from which you made a decision.

If you could put that in with the rest of your code in a form that can be rerun later at will, it will allow others to see what you did, considered and the conclusion. It will also allow others to rerun the script at a later time where things might have changed to see if one of the other approaches is now more viable, instead of just having future maintainers blindly accepting an ancient conclusion (also known as https://en.wikipedia.org/wiki/Cargo_cult_programming)

4
  • Thanks. I don't understand your suggestion. What do you mean by put that in a script? The script is already there, containing the method I chose to go with. How do I express the benchmarking that led to choosing this method over others I tested?
    – Emman
    Commented Nov 7, 2021 at 9:56
  • 1
    Well, how did you do it? Write a single program that generates the output comparing the methods you showed. Commented Nov 7, 2021 at 12:07
  • Or in the very least, a script or small program that makes an assertion about how long it should take - a performance test, essentially. Commented Nov 8, 2021 at 18:18
  • @ThorbjørnRavnAndersen very good idea. May I suggest that you update your answer to improve it and better explain what you mean by "the script" that they can rerun? (I understood what you meant, but clearly OP and others did not.)
    – user949300
    Commented Nov 9, 2021 at 22:27
1

Git is a fine place to store this alternative functionality

Either in a feature branch or as a commit in the history

Most code quality tools discourage commented out code, comments should explain code not be code itself

Some answers here encourage a bad practice, going against industry wide and heavily scrutinized code quality tools, which makes me wonder if their rationale is extremely niche or even valid in thier own niche (had they even challenged thier opinion?)

Git itself is a favourite of mine for these exact use cases, and i also quite like the idea of adding a design document as markdown in the corresponding repo or as a pull request write up

12
  • I often create a folder called "OldStuff" or "Misc" to hold such "no longer used" code. Yes, the name could be better. Depending on you and your coworkers expertise with git (I'm not a star) this may be simpler for them than a branch or named commit.
    – user949300
    Commented Nov 7, 2021 at 6:33
  • Long lived branches scales very badly. And how exactly would you document a single commit? Commented Nov 8, 2021 at 19:29
  • For all SO questions one assumes the OP doesn't (yet) have the expertise, or they'd not be asking. So what's your point? Do you post this comment on every SO answer @user949300
    – Stof
    Commented Nov 8, 2021 at 21:28
  • There's no part of the question suggesting the alternative code must be long lived, and the entire premise of using git means the changes remin in the git history not the current source, ergo you may merge the branch and if you have a perception of scale challenges but i assure you that you also don't prune the git commit depth which is equal to keeping a branch but you probably have no perception of issues with that do you @Thorbjørn Ravn Andersen??
    – Stof
    Commented Nov 8, 2021 at 21:33
  • As described, a rich documentation for the commit can be the PR, it's functionally equivalent to any other documentation method
    – Stof
    Commented Nov 8, 2021 at 21:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.