Peer Review¶
No code or documentation can be written alone. Regardless of its perceived impact, all contributions to a codebase both technical and conceptual must be reviewed by a fellow engineer before being accepted into the base branch. This is not only for the health of the product: it is also an opportunity for the team to grow, both as a unit and individually.
In this article we'll touch on the benefits of peer review and establish our principles for the process. By the end, it should be clear to both the reviewer and the reviewed what the goal of the process is and how it should be carried out. As with many other documents, there are overlaps with traditional software engineering here, but we choose to focus particularly on the machine learning perspective.
Peer reviews occur in one of three settings:
- An approach review for a new experiment.
- A code review for a completed experiment.
- A code review for operational, infrastructural, or otherwise non-experiment code.
Benefits¶
For the Author¶
In all settings, peer reviews offer the author an opportunity to improve as a software engineer.
It is unlikely that a peer review will lead to a fundamental change in the mathematical approach to a problem, but opportunities to improve in modeling techniques still exist during the peer review process. This primarily comes from the approach review, where the author's conceptual modeling of a problem and their application of mathematical techniques -- the results of their experimentation -- are scrutinized. Among the goals of the approach review are challenging the assumptions of the author, identifying gaps in their logic, and offering them an opportunity to clarify their explanations. The hope is that through repeated approach reviews over the course of many experiments, all machine learning engineers develop robust mathematical intuition, and clear communication skills when discussing deeply technical solutions.
During code reviews, as is the case with all software engineering, the author receives feedback that improves their skills as an engineer. They may learn about new and more efficient techniques from other team members, or learn how to make their code more concise and readable.
For the Reviewer¶
Peer review is a mutually beneficial exercise. It is not a one-way street.
For the reviewer, exposure to the code of others on the team can expose them to techniques they were unaware of, and ideally it demonstrates and reinforces good coding practices. At times, more can be learned about effective programming from reading than from writing. The asme is true at the mathematical level when it comes to reading and digesting the approaches of others to architecting solutions.
Communicating critiques effectively can be tricky, and this is a skill that is necessarily developed as part of being a reviewer. This is a valuable aspect of communication that benefits the entire team, especially as all in the team become comfortable in the role of reviewer.
Principles¶
For the Author¶
- Write documentation with the review in mind.
- A thought may make sense to you, but will it make sense to your reviewer? If the answer is yes, the odds have increased that your documentation will make sense broadly, to current and future audiences.
- Take care of the small details so your reviewer doesn't have to.
- Code that does not pass tests, is incorrectly formatted, is missing components, or does not follow the style guide is not code that we as an organization want reviewers to spend their time examining. Doing your best to ensure that these things are taken care of before a review shows respect for the reviewer. Tools have been put in place to help with this.
- Don't be afraid to disagree.
- Requested changes are not required changes. If you strongly feel that a comment made in your review may not be changing things for the better, respond to your reviewer and open the point for discussion by laying out a clear, respectful argument to keep the current approach.
For the Reviewer¶
- Be ruthless.
- If you have a concern during your review, no matter how small, make it into a comment. The comment may be refuted by the author, it may be acknowledged, it may make no difference to the final product at all, but no one knows until the point is raised.
- Use the review to ask questions.
- Not all comments during a review have to be points of critique or requests to change something. Peer review is the time when a piece of code is at its most visible, so this is the time to ask any questions you may have about how it works.
- When reviewing documentation, pay attention to readability.
- Channel your inner high school English teacher and break out the red pen. Documentation should be clear, concise, and free of mistakes as much as possible. If you feel a sentence can be shorter, that is a valid comment, even if it doesn't change the meaning or add any new information.
- Pay attention to test cases, doing your best to identify gaps.
- The author will surely have a better understanding of this than you, but a fundamental use of peer review is to identify areas where there may have been a mental lapse for the author, and that includes test cases. Consider the combinations of scenarios and verify that the ones you expect to see are present.
- You are the enforcer of the style guide.
- Because the majority of our style guide cannot be enforced by tooling -- hence its presence in the style guide -- it is up to the peer review process to be the enforcer of these guidelines.