Skip to content

3. Reviewing a change ​

The linter reports on a whole project; a reviewer looks at a change. fobis review reports only the findings on the lines a change touches: the uncommitted changes by default, the staged ones with --staged (a pre-commit hook), or the commits of a branch with --base master (a pull request).

A change to sweep: the number of intervals goes into a variable.

console
$ git diff
diff --git a/src/sweep.F90 b/src/sweep.F90
index 7d85740..c5396f5 100644
--- a/src/sweep.F90
+++ b/src/sweep.F90
@@ -9,11 +9,13 @@ integer,   parameter :: cases = 12                !< Number of cases.
 real(R8P), parameter :: pi = acos(-1._R8P)        !< Pi.
 real(R8P)            :: error(2, cases)           !< Errors, trapezoid and Simpson, of each case.
 integer              :: k                         !< Counter.
+integer              :: n                         !< Number of intervals.
 
 !$omp parallel do
 do k = 1, cases
-   error(1, k) = abs(trapezoid(sine, 0._R8P, pi, 2 ** k) - 2._R8P)
-   error(2, k) = abs(simpson(sine, 0._R8P, pi, 2 ** k) - 2._R8P)
+   n = 2 ** k
+   error(1, k) = abs(trapezoid(sine, 0._R8P, pi, n) - 2._R8P)
+   error(2, k) = abs(simpson(sine, 0._R8P, pi, n) - 2._R8P)
 enddo
 !$ print '(A, I0)', 'threads: ', omp_get_max_threads()
 print '(A6, 2A18)', 'n', 'trapezoid error', 'simpson error'
console
$ fobis review
src/sweep.F90:16: OMP001 [warning] 'n' is shared and written by every thread (missing private?)

1 finding(s) in 10 file(s): OMP001 x1
Explain a rule with: fobis lint --explain <ID>
Reviewed 4 changed line(s) in 1 Fortran file(s).
[exit status 1]

n is shared by the OpenMP threads, and every thread writes it: a data race. The results would still look right most of the time, which is what makes races expensive. The whole project is analysed (the rule needs the directive around the loop), but only the changed lines are reported.

For a pull request, the review is a comment:

console
$ fobis review --format markdown
### fobis review

1 finding(s) on the changed lines (1 warning), 4 changed line(s) in 1 Fortran file(s).

| Severity | Rule | Location | Message |
|---|---|---|---|
| warning | `OMP001` shared-scalar-written-in-parallel-region | `src/sweep.F90:16` | 'n' is shared and written by every thread (missing private?) |

<details><summary>Rules</summary>

- `OMP001` shared-scalar-written-in-parallel-region: A shared scalar is assigned inside an OpenMP parallel region: a data race.

Explain a rule with `fobis lint --explain <ID>`.

</details>
[exit status 1]

The fix is a private(n) clause:

console
$ fobis review
0 finding(s) in 10 file(s)
Reviewed 5 changed line(s) in 1 Fortran file(s).

fobis review and fobis review --deep on an OpenMP change, with coder-7b

coder-7b on an NVIDIA A30 of a cluster node, reached through ssh; recorded in real time.

What you learned

fobis review (working tree, --staged, --base), --format markdown. Reference: review, reviewing a change.

Next: 4. Querying the code.