Quickstart edits

[zmoon]

I'm doing a bit of an audit of the Quickstart Fortran Tutorial. So far I mostly corrected some English stuff and tried to make the code style consistent within each page (but not across all pages).

I would like to make style consistent across the whole tutorial though, but that requires deciding on some things that are currently not consistent among all pages:

• code style
  • indentation (2 or 4 spaces? or 3 or other?)
  • whether to indent module contains rt. module, similarly for type contains
  • when to put a space after , (do loop, arguments, indexing, etc.)
  • vertical aligning of :: and inline comments. Important?
  • should the base indentation level in a given markdown code block always be 0 (perhaps making it easier to copy), or should it be > 0 if the code would be normally indented if put into context (as done in some of the examples)?
  • always capitalize first word in comments?
• text style
  • when to use strong vs emphasis vs code
  • title case vs sentence case for the titles of the pages

I also have some content questions:

• what is meant by "do concurrent is not a basic feature of Fortran" in Operators and Control Flow?
• would a page on formatted printing and such fit here?

[awvwgk]

I'm personally in the 3 space indentation fraction, but we usually consider using 2 or 4 spaces here. Most important is readability w.r.t. horizontal space IMO, the code block shouldn't get clamped on the right side due to over-indented code. The code blocks on the homepage can have a width of 80 to 90 characters before horizontal scrolling is necessary in our current page layout. This shouldn't be an issue in the quickstart guide but could be an issue for other minibooks which require more complex examples.

I think either two spaces and indentation of every unit (something, end something), like used now in the quickstart guide:

module m
  implicit none
  private
  public :: a_t

  type :: a_t
    integer :: val
  contains
    procedure :: func
  end type a_t

contains

  subroutine func(self)
    class(a_t), intent(in) :: self
    if (val > 0) then
      print *, val
    end if
  end function func

end module m

Or four space indentation only for non-procedure / module units (I think this is @certik's preferred style, I usually find this hard to read):

module m
implicit none
private
public :: a_t

type :: a_t
    integer :: val
contains
    procedure :: func
end type a_t

contains

subroutine func(self)
class(a_t), intent(in) :: self
if (val > 0) then
    print *, val
end if
end function func

end module m

[zmoon]

Most important is readability w.r.t. horizontal space IMO, the code block shouldn't get clamped on the right side due to over-indented code. The code blocks on the homepage can have a width of 80 to 90 characters before horizontal scrolling is necessary in our current page layout. This shouldn't be an issue in the quickstart guide but could be an issue for other minibooks which require more complex examples.

On mobile in portrait mode, the number of viewable columns can be even less. I am seeing that longer lines get soft-wrapped, which can be a bit confusing since the rendered blocks don't currently have line numbers. Perhaps optional code block line numbers or horizontal scroll bars could be added. I think 90 is a reasonable max and inline comments shouldn't go too far over it.

[LKedward]

Many thanks for looking into this Zachary @zmoon . For most of your points, as long as the style is consistent across the tutorials then the particular choice doesn't matter too much. Re indentation, I think we should go with 2 spaces with zero base indentation for reasons already discussed, and my preference would be to indent module procedures for readability as suggested by Sebastian. I would also like to avoid vertical aligning of :: and comments since this adds maintenance burden and pushes the line length in most cases. Once you settle on a set of rules for style, would you mind documenting them as a new section in the Style Guide? This will help others keep future edits consistent.

• what is meant by "do concurrent is not a basic feature of Fortran" in Operators and Control Flow?

I'm not sure what is meant by this, please remove or reword as you see fit.

• would a page on formatted printing and such fit here?

Yes absolutely, this would make a good addition to the quickstart tutorial.

[milancurcic]

Thank you, @zmoon, all looks good, pending the feedback from @awvwgk and @LKedward.

Even when we enable horizontal scroll, we should be conservative with indentation because of small line length on mobile (it's only 30- or 40-something characters on most smartphones).

[zmoon]

Thanks @awvwgk, @LKedward, @milancurcic for the comments so far. I will draw up tutorial style guidelines for the Style Guide and then wait for feedback on that before enforcing it across the Quickstart pages.

I will do a separate PR for page on formatted print.

[certik]

Yes for tutorials let's use zero based indentation and 2 spaces, essentially spare as much as we can, so that it's reasonable on mobile also. (For my codes I prefer 4 spaces, but I think the tutorial use case is different.)

[zmoon]

Ok, I started the style guidelines (here). Feel free to comment.

What about spaces after ,? I don't think anyone commented on that yet. Currently there are many cases in the tutorial where no spaces are used after ,, e.g. for do loops, array init, printing, etc. My personal preference is to add a space after , except in indexing (a(:,:), a(1,2), do i = 1, n, b = [1, 2, 3]). I feel that this usually improves readability.

[awvwgk]

My personal preference is to add a space after , except in indexing (a(:,:), a(1,2), do i = 1, n, b = [1, 2, 3]). I feel that this usually improves readability.

Quite similar to the @dftbplus style guide here, which we could use as inspiration for examples on white space usage.

[ivan-pi]

My preference is to omit whitespace in procedure calls, e.g. call proc(a,b,c) instead of call proc(a, b, c), but I see now this is against both PEP 8 and Google C++ style guides.

[LKedward]

Style guide additions look good to me, thanks! I think your suggestion for spacing around commas seems reasonable to me.
This PR is still in draft status - do let us know when you're ready for review. Great work!

[zmoon]

@LKedward I still have a few things I want to do first but I will be sure to mark ready after that, thanks.

[zmoon]

@milancurcic could you (or someone else) explain what is meant by "do concurrent is not a basic feature of Fortran" in Operators and Control Flow?

[awvwgk]

I think the comment on do concurrent refers the difference between expectation and actual implementations in compilers. I'll try to summarize what I picked up from the threads in discourse:

• concurrent doesn't mean parallel, yet most users mean parallel when they are using do concurrent
• from a strict viewpoint, there is no guarantee that a do concurrent can actually be run in parallel
  • the compiler must ensure that the loop is correctly run sequentially if no parallelization due to data dependencies is possible
  • practically this kind of compile time checks are difficult, if not impossible to implement (calling a pure procedure)
• some compiler implement the do concurrent as a parallel do loop regardless, even if there is a data dependency which could prevent parallelization
• some of those short-comings have been fixed in the Fortran 2018 variant of do concurrent which allows to declare the data dependencies explicitly

[zmoon]

I think the comment on do concurrent refers the difference between expectation and actual implementations in compilers.

So maybe the first sentence in the note, "do concurrent is not a basic feature of Fortran.", could be replaced by something like "Don't expect a loop to be parallelized simply by changing from do to do concurrent". And I think also could mention compiler flags are needed, yes?

Is there a resource comparing what the different major compilers can/will do with it?

[awvwgk]

Maybe we can just put a general statement there that concurrency does not imply parallel execution. Not sure what GFortran does, but ifort will parallelize do concurrent if the -qopenmp flag is set, nvfortran will parallelize with -stdpar=multicore and generate device offloading code with -stdpar=gpu.

[zmoon]

Cool. Yeah I just feel since it's for the Quickstart we should mention that compiler flags are necessary to get parallelization, and maybe mention a few (maybe should be gfortran (I guess would be -ftree-parallelize-loops=n + an -O flag ≥ 1; not sure if it does anything with -fopenmp) since it is used elsewhere in the minibook) / link to an external resource with more details, to give people what they need to get started with it quickly.

[zmoon]

@awvwgk I made some changes to the do concurrent note but feel free to suggest others

[zmoon]

@LKedward do you want to review?

[LKedward]

Apologies for the delay in reviewing - this is great work @zmoon! A significant improvement and it's great to have a set guidelines for styling future mini-books. Very minor, but you could mention the use of the Oxford comma in the Text section of the style guide for future consistency. Other than that I haven't spotted any issues with these changes. Many thanks!

[LKedward]

#build_preview

[github-actions]

This PR has been built with Jekyll and can be previewed at: https://fortran-lang.org/pr/255/

[zmoon]

@LKedward were you able to look at those small CSS changes and their impact? It adds horizontal scroll bars for the code blocks e.g. for mobile. And I was trying to fiddle with the container width a bit, in regards to #256.

[LKedward]

Sorry @zmoon, yes those CSS updates look fine to me and the rendering seems to work better now for narrow screens 👍

[awvwgk]

With three approvals I will go ahead and merge this patch.

[awvwgk]

#delete_preview

[github-actions]

The preview build for this PR has now been deleted.