Update names based on new naming convention with prif and caffeine
#45
Conversation
`prif` versus `caf` and `caffeine`.
src/prif_m.f90
Outdated
| use program_termination_m, only : prif_stop, prif_error_stop | ||
| use image_enumeration_m, only : prif_this_image, prif_num_images | ||
| use collective_subroutines_m, only : prif_co_sum, prif_co_max, prif_co_min, prif_co_reduce, prif_co_broadcast | ||
| use caffeinate_decaffeinate_m, only : prif_caffeinate, prif_decaffeinate |
There was a problem hiding this comment.
There is no specified prif_caffeinate nor prif_decaffeinate. There is only prif_init
src/prif_m.f90
Outdated
| @@ -0,0 +1,11 @@ | |||
| ! Copyright (c), The Regents of the University of California | |||
| ! Terms of use are as specified in LICENSE.txt | |||
| module prif_m | |||
There was a problem hiding this comment.
@everythingfunctional This PRIF module name is not currently part of the spec draft, but IIUC it probably should be.
How common is this _m suffix convention? Is prif_m what we should adopt?
There was a problem hiding this comment.
_m is a common enough convention, but I don't think it's needed in this context. I think the entire interface should be defined in a single module named prif. I'll add it to the design doc.
There was a problem hiding this comment.
Actually, it was already there.
The procedures and types provided
for direct invocation as part of the runtime library shall be defined in a
Fortran module with the nameprif.
|
|
||
| interface caf_num_images | ||
| interface prif_num_images |
There was a problem hiding this comment.
prif_num_images is currently specified as one function with two optional arguments.
| @@ -24,17 +24,17 @@ module function num_images_team_number(team_number) result(image_count) | |||
|
|
|||
| end interface | |||
|
|
|||
| interface caf_this_image | |||
| interface prif_this_image | |||
There was a problem hiding this comment.
prif_this_image in the draft spec has a very different generic subroutine interface (including different argument order). This is probably not the only conformance problem (even ignoring the unimplemented interfaces), just one I happened to notice.
I realize this PR is focused on renaming, do you want to do a separate pass for interface conformance?
example/hello.f90
Outdated
| implicit none | ||
|
|
||
| if (caf_caffeinate() /= 0) error stop "caffeinate returned a non-zero exit code" | ||
| if (prif_caffeinate() /= 0) error stop "caffeinate returned a non-zero exit code" |
There was a problem hiding this comment.
Shouldn't this be prif_init?
|
@bonachea, @everythingfunctional. Thanks for the reviews, I will try to address all comments here, since some are of a similar nature and all of my responses are relevant to everyone (@rouson, feel free to read the above review comments and my responses here). I will change the My plan for this PR was to only change names of the existing procedures to match the new naming convention, but not change anything else. The interfaces here are not consistent with our design doc interfaces, but I would prefer to change the names in one PR and then work bit by bit in later PRs to update the interfaces. The goal is to get to the point that Damian and I can start to work on hooking up a memory allocator in Caffeine. So I would prefer not to change any procedure interfaces in this PR. But I am happy to change |
Sounds fine.
Despite the amusing name, I don't think |
| void caf_c_caffeinate(int argc, char *argv[]); | ||
| void caf_c_decaffeinate(int exit_code); | ||
| void caf_caffeinate(int argc, char *argv[]); | ||
| void caf_decaffeinate(int exit_code); |
There was a problem hiding this comment.
prif_stop calls caf_decaffeinate. Should the caffeine side names be updated as well to caf_init and caf_stop to represent more closely the relationship with prif_init and prif_stop?
There was a problem hiding this comment.
I'm not too worried about the names of "implementation" procedures. I would envision an implementation doing, for the sake of reducing procedure call overhead, something like
module prif
use caffeine, only: &
prif_init => caffeinate, &
prif_stop => decaffeinate, &
...
end module
At least to the extent that the interfaces match up.
No description provided.