Consistent use: Multiple occurances - Inconsistent Function Definitions

[wesbland]

Problem

Some function definitions are missing or have inconsistent (with similar arguments in other functions) descriptions. See, for example, MPI_TESTALL (page 62) description for the flag argument should use the same text as MPI_TESTANY (page 60), except "true if all..."

Suggested Fix

Suggest asking the bindings committee to review

[wesbland]

Needs scoping, but maybe chapter committee:

• Editor - 2
• Chapter committee - 3
• Errata - 1
• Full Proposal - 1

[wesbland]

Each committee chair should check the box when they're done. Creating a small pull request for each chapter (or group of chapters if you own more than one) is probably as good a way as any to do this. Make sure that in your pull request, you mention this ticket (#280) without saying "Fixes" to avoid automatically closing it since there will be more than one PR to do this work. Also, if there's nothing to be done in your chapter, just check the box.

• Front matter - @wgropp
• Introduction - @wgropp
• Terms - @cblaas
• Point-to-Point - @dholmes-epcc-ed-ac-uk
• Datatypes - @bosilca
• Collectives - @tonyskjellum
• Communicators - @GuillaumeMercier
• Topologies - @RolfRabenseifner
• Partitioned Communication - @regrant
• Environment - @bosilca
• Info - @schulzm
• Process Creation - @hppritcha
• RMA - @wgropp
• External Interfaces - @schulzm
• I/O - @tonyskjellum
• Tools - @mahermanns
• Deprecated - @RolfRabenseifner
• Removed - @RolfRabenseifner
• Backward Incompatibilities - @wesbland
• Bindings - @puribangalore
• Bindings Summary - @RolfRabenseifner
• Change Log - @RolfRabenseifner

@martinruefenacht will add an automated check to look for these empty descriptions with no default description and flag them, causing the check to fail. The chapter committees will need to fix those.

[martinruefenacht]

Currently when the binding tool comes across a parameter with an empty description a default description mapping is looked up.

These default descriptions were taken from MPI 3.1:

KIND : DESCRIPTION
ERROR_CODE : error code
ERROR_CLASS : error class
COMMUNICATOR : communicator
ERRHANDLER : MPI error handler
INFO : info argument
FILE : file
REQUEST : communication request
STATUS : status object
WINDOW : window object
ASSERT : program assertion

If none is found then we just leave it as empty with the LIS typing.

For empty description parameters we can either introduce a default description for that KIND or we can set the "desc" attribute of that parameter.

For the given example "flag" we would set "desc" since it is not a global description.

[martinruefenacht]

@wesbland Does this prohibit empty LIS descriptors? (easy to test for with the default descriptions)

The notion of a similar/related function with similar parameters and "inconsistent" descriptions will be difficult to test. It could be done with not so complex to complex methods + a threshold (dicey).

But until we have semantic information via semantics-wg we can't do this well unless we label related functions, which I think defeats the purpose.

[GuillaumeMercier]

Question: in chapter 6, I see this for the definition of MPI_Comm_idup:

\cdeclindex{MPI\_Comm}%                                                                                                                                                                       
\begin{mpi-binding}
    function_name("MPI_Comm_idup")

    parameter("comm", "COMMUNICATOR", desc="communicator")
    parameter( 
        "newcomm",
        "COMMUNICATOR",
        direction="out",
        asynchronous=True,
        desc="copy of \\mpiarg{comm}",
    )

Is it normal to have \\mpiarg{comm} instead of \mpiarg{comm} ?
I can see the same thing in a bunch of places throughout the chapter but in other places , only a single \ (e.g, MPI_Group_incl) for \mpiarg (for instance).

Thanks.

[dholmes-epcc-ed-ac-uk]

Is it normal to have \\mpiarg{comm} instead of \mpiarg{comm} ?

This is LaTeX code within a Python string; the double \ is Python's way of escaping a single \ that would otherwise be interpreted as a special character.

See, for example, https://www.pitt.edu/~naraehan/python2/tutorial7.html

[GuillaumeMercier]

Yes, but there are a lot of places with a single \ too. And the resulting pdf is fine (which seems to mean that the single \ is interpreted right). Hence my question.

[GuillaumeMercier]

Just to be clear: I don't care about \\ or \ but rather about consistency. I'm fine with either way.

[dholmes-epcc-ed-ac-uk]

Yeah, ok; following several usages of grep on the *.tex files and then on the *.py files plus looking at the pdf and a few lines of the Python code, I see that the single \ works but I'm not sure why.

My best guess is that "\m" is not a recognised Python escape sequence, i.e. Python has no other way to interpret this except as two normal characters, so it is kind and does the sensible thing. Same for "\c" (used for the "\const" command) and others. That is, the correct/intended evaluation of a single \ is an accident.

This guess seems to be backed up by this reference page: https://docs.python.org/2.0/ref/strings.html

If so, then LaTeX commands like '\annexref{' (used in chap-topol but, fortunately, not in a binding) and '\begin{' (used all over the place but, fortunately, not in bindings) would go wrong. Fortunately, we are reserving use of '\ftype' (and similar commands beginning with f) in bindings to the LaTeX code output by the Python, rather than permitting it/them in the input Python code.

Maybe @martinruefenacht will be able to explain (better).

If my guess is right, then we should decide between "always escape " and "always escape \ when it matters" as a style rule and implement a make check to enforce consistency.

[martinruefenacht]

@GuillaumeMercier Yes, the "\\" is the correct way of expressing a "\" (not talking about raw strings right now). There is no "\m" character therefore it works either way, but for correctness/consistency we should be using "\\" as you said (PEP8 python style guide requires proper escaping).

I thought I had gone through the mpi-binding blocks and did "\" -> "\\". I will write a test for it... (what I should have done at the time)

If you are referring to "\" outside the mpi-blocking blocks, that is how it is meant to be, since that is Latex and not Python code.

[GuillaumeMercier]

f you are referring to "" outside the mpi-blocking blocks, that is how it is meant to be, since that is Latex and not Python code.

I'm only referring to Python code, of course.

[wesbland]

@wesbland Does this prohibit empty LIS descriptors? (easy to test for with the default descriptions)

I think that was the idea of what we discussed on the call on Wednesday, yes. You can only have an empty LIS description if there is a default description that will fill the gap.

[GuillaumeMercier]

@martinruefenacht and @dholmes-epcc-ed-ac-uk : OK, that makes sense (as to why \ is working).
Do you want me to make a pass in chapter 6 to fix this (as a part of my PR)?

[martinruefenacht]

@GuillaumeMercier I think I should combine a pass over the document and a test for it in a single PR. Unless you definitely want to do it.

[GuillaumeMercier]

@GuillaumeMercier I think I should combine a pass over the document and a test for it in a single PR. Unless you definitely want to do it.

Not particularly. I prefer your global approach.

[omor1]

re: string-escape issue, it probably makes more sense (and is easier) to use a raw string:

\cdeclindex{MPI\_Comm}%
\begin{mpi-binding}
    function_name("MPI_Comm_idup")
    
    parameter("comm", "COMMUNICATOR", desc="communicator")
    parameter(
        "newcomm",
        "COMMUNICATOR",
        direction="out",
        asynchronous=True,
        desc=r"copy of \mpiarg{comm}",
    )
    parameter(
        "request", "REQUEST", direction="out", desc="communication request"
    )
\end{mpi-binding}

@dholmes-epcc-ed-ac-uk you are correct that Python's default way of interpreting unrecognized escape sequences is to ignore it; as of Python 3.6 this is deprecated, with the intent of eventually changing this into an error.

[martinruefenacht]

@omor1 Agreed. Either works, but it seems to be causing misunderstandings. I will work on the binding_linter so simply convert all the strings we have to raw-strings.

[GuillaumeMercier]

Just to be sure: should I convert the \\ into \ in my PR?

[martinruefenacht]

@GuillaumeMercier No, let's do those changes in a separate pass given this issue/PRs are intended to fix inconsistent descriptions or defaulted (by setting it empty).

[bosilca]

PR mpi-forum/mpi-standard/pull/342 addresses these concerns for the datatype chapter. Nothing needed to be amended in the environment chapter.