Transferring fortran90.org "Fortran Best Practices" into a mini-book #246

vmagnin · 2021-04-23T19:42:51Z

Transferring the https://www.fortran90.org/src/best-practices.html page, into a mini-book named Fortran Best Practices.
See discussion in issue fortran-lang/webpage#112

A preview of the minibook can be found at https://fortran-lang.org/pr/246/learn/best_practices

jvdp1 · 2021-04-23T20:07:12Z

Great and valuable content. Thanky you. Is it ready for review?

awvwgk · 2021-04-23T20:12:44Z

The overall structuring looks quite good, but some chapters like integer division and the parallel programming seem very short. For other chapters I'm not sure if they are really suited in a best practice minibook, like interfacing with Python probably deserves its own minibook at some point.

I haven't looked over the content in detail yet but already spotted a few issues like broken links, typos, inconsistencies. If I find some time I can give it an editing pass. If this is what we want to do next here.

learn/best_practices/arrays.md

certik · 2021-04-23T20:20:56Z

This looks great, thank you @vmagnin! Let us know what your plan is. If it is ready for review, we can go over and clean it up. Also let's think about the wording, I already have some ideas for making it more polished and less opinionated, but still pretty clear recommendation how to program in modern Fortran.

vmagnin · 2021-04-24T12:49:56Z

My personal objective was to work on the transfer of the code from fortran90.org to Fortran-lang.org (RST => Markdown mini-book).

The community can now review and edit the content in order to integrate it better with the rest of the site.

For example, concerning parallel programming, probably it should have its own mini-book, developing OpenMP and MPI, and adding Coarrays.

But I guess we should now define what must be done now before accepting the PR and putting the content online, and what could be done later.

vmagnin · 2021-04-25T19:15:28Z

some chapters like integer division and the parallel programming seem very short.

The "Integer division" chapter could be easily enriched into a more general "Integer numbers" and talk about:

the different kinds of integers (and kinds defined in iso_fortran_env),
and their ranges and the problems when you go out of range: beginners knowing only Python may not be aware of those problems, as in Python the number of bytes is automatically adapted as needed... No limit!
inquiry functions like huge(),
the fact that there is no unsigned integers (perhaps in Fortran 202x?) in Fortran, a big difference for people knowing C,
...

awvwgk

I made a quick indentation pass to change from 4 or 3 space indentation to 2 space indentation similar like in the quickstart guide. Also, I fixed a few minor things I found while going through.

learn/best_practices/index.md

awvwgk · 2021-05-08T10:05:52Z

learn/best_practices/style_guide.md

+Indentation
+-----------


Should we have an opinion on tabs in Fortran here?

awvwgk · 2021-05-08T10:06:43Z

learn/best_practices/type_casting.md

+transfer() Intrinsic Function
+-----------------------------
+
+Before Fortran 2003, the only way to do type casting was using the
+`transfer` intrinsic function. It is functionally equivalent to the
+method V, but more verbose and more error prone. It is now obsolete and
+one should use the method V instead.
+
+Examples:
+
+<http://jblevins.org/log/transfer>
+
+<http://jblevins.org/research/generic-list.pdf>
+
+<http://www.macresearch.org/advanced_fortran_90_callbacks_with_the_transfer_function>


This doesn't look good at all, either we describe it ourselves or we remove this section completely.

awvwgk · 2021-05-08T10:07:22Z

learn/best_practices/type_casting.md

+Complete Example of void \* vs type(c\_ptr) and transfer()
+----------------------------------------------------------
+
+Here are three equivalent codes: one in C using `void *` and two codes
+in Fortran using `type(c_ptr)` and `transfer()`:
+
+| Language &nbsp; | Method               | Link                              |
+|-----------------|----------------------|-----------------------------------|
+| C               | `void *`             | <https://gist.github.com/1665641> |
+| Fortran         | `type(c_ptr)` &nbsp; | <https://gist.github.com/1665626> |
+| Fortran         | `transfer()`         | <https://gist.github.com/1665630> |
+
+The C code uses the standard C approach for writing extensible libraries
+that accept callbacks and contexts. The two Fortran codes show how to do
+the same. The `type(c_ptr)` method is equivalent to the C version and
+that is the approach that should be used.
+
+The `transfer()` method is here for completeness only (before Fortran
+2003, it was the only way) and it is a little cumbersome, because the
+user needs to create auxiliary conversion functions for each of his
+types. As such, the `type(c_ptr)` method should be used instead.


Can't we just move the code snippets here? Linking against gist doesn't really look nice IMO.

jvdp1

Here are some comments. IMO some sections should not be in a "Best Practices" section (e.g., parallel programming, ...)

learn/best_practices/arrays.md

learn/best_practices/c_interfacing.md

learn/best_practices/arrays.md

jvdp1 · 2021-05-08T17:48:18Z

learn/best_practices/type_casting.md

+Point is: it needn't be "one abstract type to encompass all" or bust.
+There are natural and viable options between "all" and "none".
+
+Private Module Variables


Suggested change

Private Module Variables

III) Private Module Variables

jvdp1 · 2021-05-08T17:48:34Z

learn/best_practices/type_casting.md

+cleanest way. However, with a bit of thought, usually there is a better,
+safer, more explicit way along the lines of II or IV.
+
+Nested functions


Suggested change

Nested functions

IV) Nested functions

jvdp1 · 2021-05-08T17:49:00Z

learn/best_practices/type_casting.md

+end subroutine foo
+```
+
+Using type(c\_ptr) Pointer


Suggested change

Using type(c\_ptr) Pointer

V) Type(c\_ptr) Pointer

jvdp1 · 2021-05-08T17:49:41Z

learn/best_practices/type_casting.md

+other interface elements), simplest, least bug-prone, and fastest is to
+use one of the previous approaches.
+
+transfer() Intrinsic Function


Suggested change

transfer() Intrinsic Function

VI) transfer() Intrinsic Function

jvdp1 · 2021-05-08T17:50:04Z

learn/best_practices/type_casting.md

+
+<http://www.macresearch.org/advanced_fortran_90_callbacks_with_the_transfer_function>
+
+Object Oriented Approach


Suggested change

Object Oriented Approach

VII) Object Oriented Approach

- add discussion of assumed-rank and assumed-size arrays

Co-authored-by: Jeremie Vandenplas <[email protected]>

awvwgk · 2021-06-02T13:49:25Z

arjenmarkus · 2021-06-02T13:57:00Z

I merely scanned the booklet, but I noticed a "gotcha" regarding floating-point numbers: it looked like a link but it does not lead anywhere. And in the chapter on parallel computing I missed a section on coarrays. But it is taking shape and that is an important thing :) Op wo 2 jun. 2021 om 15:49 schreef Sebastian Ehlert < ***@***.***>:

…

I extended and rewrote a couple of chapters. There is still a lot to do and I haven't gotten around to check every chapter. I would also suggest to remove at least three of the currently added chapters in favor of separate minibooks. - Introduction - Fortran Style Guide (sync with updated style guide from fortran-lang/fortran-lang.org#255 <#255>) - Floating Point Numbers - Integer Division - Modules and Programs - Arrays - Multidimensional Arrays - Element-wise Operations on Arrays Using Subroutines/Functions - Allocatable Arrays - File Input/Output - Interfacing with C (remove in favor of separate minibook?) - Interfacing with Python (remove in favor of fortran-lang/webpage#105 <fortran-lang/webpage#105>?) - Callbacks - Type Casting in Callbacks - Parallel programming (remove in favor of separate minibook?) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#246 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN6YR4SGOASYO66UY6BYQLTQYZHJANCNFSM43PHAVYA> .

- remove mention of impure newunit function hack

awvwgk · 2021-06-03T10:06:14Z

I'm done for now with editing, the remaining chapters still need some work, though.

The typecasting in callbacks chapter is quite a mess and I didn't really grasp what message should be conveyed there, but the external links against gists and blogs are unacceptable IMO, if we can't explain it in the minibook we shouldn't mention it at all. Also, the elemental procedure chapter is really fuzzy and I didn't really got the overall message there, probably requires rewriting from scratch.

awvwgk · 2021-06-08T21:03:12Z

Updating the #build_preview again.

github-actions · 2021-06-08T21:05:51Z

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

awvwgk · 2021-06-17T20:34:56Z

How do want to move forward here?

There are two chapters remaining that require an editing pass. It doesn't have to be a full rewrite, but at least some cleanup should be done. The rest of the minibook should be fine for review now. I see @LKedward and @milancurcic have volunteered for reviewing already.

certik · 2021-06-17T20:55:31Z

Thanks for going over and polishing it @awvwgk. Overall I think the changes make it "less opinionated" which is I think better.

I have two comments:

in https://fortran-lang.org/pr/246/learn/best_practices/multidim_arrays it has been lost that you really want to loop over indices from the left for performance: https://www.fortran90.org/src/best-practices.html#multidimensional-arrays, so this information should be added there. The original has examples of this. We can perhaps add a subsection Performance and put this there.
The IO section seems more confusing: https://fortran-lang.org/pr/246/learn/best_practices/file_io than the original: https://www.fortran90.org/src/best-practices.html#file-input-output. Is the original wrong? I noticed the action keyword argument was added --- is that required by the standard or is it automatic? The three basic and most frequent operations are reading, writing and appending, so we can add such 3 subsections, and then we can add "other" and put the rest of the info there.

Otherwise the changes are fine with me.

brianconcannon2 · 2021-06-27T11:27:35Z

I would like to join your review if you need me. I have used Fortran since the year 1966 and am especially familiar with old Fortran code. Do you have any use for that expertise?

certik · 2021-06-27T20:16:56Z

@brianconcannon2 yes, we definitely do! Thank you. If you find any improvements in the tutorial, please let us know. (Or if you have any other questions.)

milancurcic · 2021-06-28T22:39:18Z

Thank you @brianconcannon2, I just added you as a reviewer.

awvwgk · 2021-09-04T13:13:18Z

This patch has become stale for quite some time now. Since we already put quite some effort into it I would like to move it forward.

Picking this project up again requires quite some time and energy and I only see two minor comments to address so far. Generally I think for a huge patch like this it would be useful to provide comments in Files Changed preferably together with a suggestion. Also, we don't need a complete review of the whole minibook, reviewing a chapter at a time should be sufficient to get this project moving again.

certik · 2021-09-04T15:46:23Z

If we are all more or less ok with how it looks, then I suggest we simply merge it and be done with it.

Then we can start sending many subsequent PRs to fix up little things that we find.

milancurcic · 2021-09-04T16:01:43Z

I agree, it would take a long time to review and edit such a large PR. Case in point, I still haven't reviewed it. Let's just merge it and improve with smaller PRs, as @certik suggested.

vmagnin · 2021-09-04T17:16:49Z

I agree with @certik and @milancurcic ,
once online, it will be easier to review it (and by more people once announced on the Discourse), and I think we will gain a new motivation for that task, knowing it's online!

certik

It's good enough from me to go in. We can further keep polishing this once merged.

awvwgk · 2021-09-05T09:34:53Z

Alright, than I'll go ahead and merge this PR. Thanks everybody for the effort put into this project.

vmagnin added 2 commits April 21, 2021 11:10

Transfer fortran90.org Best Practices content (#79)

6f3648d

Removed vestigial best_practices.md (2020-04-15)

b713a9b

This comment has been minimized.

Sign in to view

certik reviewed Apr 23, 2021

View reviewed changes

learn/best_practices/arrays.md Outdated Show resolved Hide resolved

certik reviewed Apr 23, 2021

View reviewed changes

learn/best_practices/arrays.md Outdated Show resolved Hide resolved

vmagnin added 2 commits April 24, 2021 14:25

Replaced HTML code by Markdown italics *...*

16b65a5

Typo: replaced 3 'enddo' by 'end do'

1a256d3

LKedward self-requested a review April 26, 2021 07:32

milancurcic self-requested a review May 1, 2021 16:03

awvwgk added 2 commits May 8, 2021 11:50

Consistent indentation (2 spaces) with quickstart minibook

6c15dea

Minor adjustments, wording, stop -> error stop

c84e3f0

awvwgk reviewed May 8, 2021

View reviewed changes

jvdp1 reviewed May 8, 2021

View reviewed changes

awvwgk and others added 6 commits June 1, 2021 11:39

Extend the alloctable arrays section

3383039

Expand chapter on arrays

976fd62

- add discussion of assumed-rank and assumed-size arrays

Expand chapter on integer division

c358ace

Add definition of callback

4114262

Co-authored-by: Jeremie Vandenplas <[email protected]>

Add intent in callback snippet

4b3de58

Co-authored-by: Jeremie Vandenplas <[email protected]>

Rework multidimensional array section

f146342

This comment has been minimized.

Sign in to view

Rework floating point chapter

b885654

Rework file IO section

158a970

- remove mention of impure newunit function hack

This comment has been minimized.

Sign in to view

awvwgk added 4 commits June 3, 2021 11:16

Rework module and program chapter

a8da64c

Update authors and introduction for best practice minibook

5cf80cc

Fix typo

5a86e10

Rework callback section in best practise minibook

c1e492a

awvwgk added help wanted Extra attention is needed learn Related to the learning resources labels Jun 5, 2021

milancurcic requested a review from brianconcannon2 June 28, 2021 22:38

milancurcic approved these changes Sep 4, 2021

View reviewed changes

certik approved these changes Sep 5, 2021

View reviewed changes

awvwgk removed the help wanted Extra attention is needed label Sep 5, 2021

This comment has been minimized.

Sign in to view

awvwgk merged commit e3373ca into fortran-lang:master Sep 5, 2021

vmagnin deleted the fortran90org branch September 5, 2021 10:12

	transfer() Intrinsic Function
	VI) transfer() Intrinsic Function


		<http://www.macresearch.org/advanced_fortran_90_callbacks_with_the_transfer_function>

		Object Oriented Approach

		Indentation
		-----------

Transferring fortran90.org "Fortran Best Practices" into a mini-book #246

Transferring fortran90.org "Fortran Best Practices" into a mini-book #246

Conversation

vmagnin commented Apr 23, 2021 • edited by awvwgk Loading

This comment has been minimized.

This comment has been minimized.

jvdp1 commented Apr 23, 2021

awvwgk commented Apr 23, 2021

certik commented Apr 23, 2021

vmagnin commented Apr 24, 2021 • edited Loading

vmagnin commented Apr 25, 2021

awvwgk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jvdp1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

awvwgk commented Jun 2, 2021 • edited Loading

arjenmarkus commented Jun 2, 2021 via email

This comment has been minimized.

This comment has been minimized.

awvwgk commented Jun 3, 2021

awvwgk commented Jun 8, 2021

github-actions bot commented Jun 8, 2021

awvwgk commented Jun 17, 2021

certik commented Jun 17, 2021

brianconcannon2 commented Jun 27, 2021

certik commented Jun 27, 2021

milancurcic commented Jun 28, 2021

awvwgk commented Sep 4, 2021

certik commented Sep 4, 2021

milancurcic commented Sep 4, 2021

vmagnin commented Sep 4, 2021

certik left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

awvwgk commented Sep 5, 2021

vmagnin commented Apr 23, 2021 •

edited by awvwgk

Loading

vmagnin commented Apr 24, 2021 •

edited

Loading

awvwgk commented Jun 2, 2021 •

edited

Loading