Skip to content

Update hook component to use enum MCA parameter #8313

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jan 11, 2021

Conversation

jjhursey
Copy link
Member

@jjhursey jjhursey commented Dec 22, 2020

  • --mca ompi_display_comm
    • mpi_init : Display during MPI_Init
    • mpi_finalize : Display during MPI_Finalize

@jjhursey
Copy link
Member Author

This was a community request to tighten up the MCA parameter before v5.

@jjhursey
Copy link
Member Author

jjhursey commented Dec 22, 2020

Here is the error message when you get the option wrong:

--------------------------------------------------------------------------
An invalid value was supplied for an enum variable.

  Variable     : comm_method
  Value        : mpi-init
  Valid values : 1:"init", 1:"mpi_init", 2:"finalize", 2:"mpi_finalize", 3:"all"
--------------------------------------------------------------------------

@jjhursey jjhursey force-pushed the prot-enum branch 2 times, most recently from 8ee5508 to 5df7f98 Compare January 4, 2021 21:46
@hjelmn
Copy link
Member

hjelmn commented Jan 4, 2021

Another option is the flag enumerator. That would allow it to take comma-delimited values such as "init,finalize".

@jjhursey
Copy link
Member Author

jjhursey commented Jan 5, 2021

@hjelmn That sounds interesting. I like that more than all. Let me play around with it a bit - I'll push a new commit for review.

@hjelmn
Copy link
Member

hjelmn commented Jan 5, 2021

Yeah, I created it because of btl_*_flags. I could never remember the flags for put, get, etc. Not we can do --mca btl_ugni_flags send,put,get and the like.

@jjhursey
Copy link
Member Author

jjhursey commented Jan 5, 2021

I just pushed a new commit to the branch that uses the enum flags instead of just an enum. I also took out the '1' and '2' options since they are not really useful for this option (there are more verbose options).

--------------------------------------------------------------------------
An invalid value was supplied for an enum variable.

  Variable     : comm_method
  Value        : 12
  Valid values : Comma-delimited list of:  0x1:"none", 0x2:"init", 0x2:"mpi_init", 0x4:"finalize", 0x4:"mpi_finalize"
--------------------------------------------------------------------------

Something odd (not related to this PR) is that the ompi_comm_method MCA parameter is not shown in ompi_info - possibly because it is not registered to a specific component?

@jsquyres
Copy link
Member

jsquyres commented Jan 5, 2021

I think you might want to remove the duplicate value. I.e., pick init / finalize or mpi_init / mpi_finalize -- don't have both.

In this mode of MCA values, those become distinct / different values.

@jjhursey
Copy link
Member Author

jjhursey commented Jan 5, 2021

I don't think I understand - they are aliases for each other

0x2:"init", 0x2:"mpi_init"
0x4:"finalize", 0x4:"mpi_finalize"
``
So `init` and `mpi_init` are both the value `0x2`

@jjhursey
Copy link
Member Author

jjhursey commented Jan 5, 2021

I see it now. There is a check in mca_base_var_enum.c:400 that checks to:

/* ensure flags are only set a single bit, doesn't conflict with itself, and
 * hasn't already been specified. */

Since I was using a debug build it didn't trip when I tested (and worked fine). I took out the duplicates leaving only mpi_init and mpi_finalize

@jjhursey jjhursey force-pushed the prot-enum branch 2 times, most recently from 9ffb024 to bac67d7 Compare January 6, 2021 15:51
@jjhursey
Copy link
Member Author

jjhursey commented Jan 6, 2021

I fixed the ompi_info display issue and squashed everything down. This is ready to go if anyone wants to take a second look.

           MCA ompi base: parameter "ompi_comm_method" (current value: "none", data source: default, level: 3 user/all, type: unsigned_int)
                          Enable the communication protocol report: when MPI_INIT is invoked (using the 'mpi_init' value) and/or when MPI_FINALIZE is invoked (using the 'mpi_finalize' value).
                          Valid values: Comma-delimited list of:  0x1:"none", 0x2:"mpi_init", 0x4:"mpi_finalize"

@jjhursey
Copy link
Member Author

jjhursey commented Jan 8, 2021

Here is how the MCA parameters show up in ompi_info now:

shell$  ompi_info --all -l 9 | grep -A4 "hook comm_method\|ompi_display_comm"
    MCA hook comm_method: ---------------------------------------------------
    MCA hook comm_method: parameter "hook_comm_method_verbose" (current value: "0", data source: default, level: 9 dev/all, type: int)
    MCA hook comm_method: parameter "hook_comm_method_display" (current value: "none", data source: default, level: 3 user/all, type: unsigned_int, synonyms: ompi_display_comm)
                          Enable the communication protocol report: when MPI_INIT is invoked (using the 'mpi_init' value) and/or when MPI_FINALIZE is invoked (using the 'mpi_finalize' value).
                          Valid values: Comma-delimited list of:  0x1:"none", 0x2:"mpi_init", 0x4:"mpi_finalize"
    MCA hook comm_method: parameter "hook_comm_method_max" (current value: "12", data source: default, level: 3 user/all, type: int)
                          Number of hosts for which to print unabbreviated 2d table of comm methods.
    MCA hook comm_method: parameter "hook_comm_method_brief" (current value: "0", data source: default, level: 3 user/all, type: int)
                          Only print the comm method summary, skip the 2d table.
    MCA hook comm_method: parameter "hook_comm_method_fakefile" (current value: "", data source: default, level: 3 user/all, type: string)
                          For debugging only: read comm methods from a file

 * `--mca ompi_display_comm VALUE` where `VALUE` is one or more of:
   - `mpi_init` : Display during `MPI_Init`
   - `mpi_finalize` : Display during `MPI_Finalize`
 * hook/comm_method: Use enum flags to select protocols

Signed-off-by: Joshua Hursey <jhursey@us.ibm.com>
@jjhursey
Copy link
Member Author

jjhursey commented Jan 8, 2021

Per offline discussion removed the 'none' option since not specifying the parameter is really 'none' anyway.

    MCA hook comm_method: parameter "hook_comm_method_display" (current value: "", data source: default, level: 3 user/all, type: unsigned_int, synonyms: ompi_display_comm)
                          Enable the communication protocol report: when MPI_INIT is invoked (using the 'mpi_init' value) and/or when MPI_FINALIZE is invoked (using the 'mpi_finalize' value).
                          Valid values: Comma-delimited list of:  0x1:"mpi_init", 0x2:"mpi_finalize"

@jjhursey jjhursey merged commit 8c89e3c into open-mpi:master Jan 11, 2021
@jjhursey jjhursey deleted the prot-enum branch January 11, 2021 19:29
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants