Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

confusingly named variables in MPI backend #36

Open
davidklaftenegger opened this issue Feb 27, 2020 · 2 comments
Open

confusingly named variables in MPI backend #36

davidklaftenegger opened this issue Feb 27, 2020 · 2 comments

Comments

@davidklaftenegger
Copy link
Member

As @lundgren87 noticed in #35 there are still many places where variables have confusing names. This issue is attempting to track instances of this as people find them while reading the code, so that information is easily found again later.

Fixing these variable names is a low priority, and it is not expected to be done all in one giant pull request, so feel free to

  • add links to code where you think this issue applies
  • create pull requests for fixing any of the things mentioned in this issue
@lundgren87
Copy link
Member

lundgren87 commented Feb 28, 2020

Getting/Setting/Using Node ID

unsigned int getID(){
return workrank;
}
unsigned int argo_get_nid(){
return workrank;
}

getID() and argo_get_nid() do the same thing. getID() is used internally, while argo_get_nid() is called externally. I believe there should be no need for both and that they could be replaced by for example get_node_id().

/** @brief rank/process ID in the MPI/ArgoDSM runtime*/
int rank;
/** @brief rank/process ID in the MPI/ArgoDSM runtime*/
int workrank;

rank and workrank denote the same thing but are set differently, and rank is redundant as it is not used outside of the following line:
MPI_Comm_rank(MPI_COMM_WORLD,&rank);

workrank and getID() are both used frequently in the code. workrank primarily in MPI calls, and getID() as part of calculations/checks. The difference between these is that getID() returns workrank as an unsigned int, while workrank is signed. It would be beneficial to standardize this and use one value where signed integers are not specifically required (MPI_Comm_rank and MPI_Group_rank).

@lundgren87
Copy link
Member

Variables id and invid

unsigned long id = 1 << getID();
unsigned long invid = ~id;

(and a few other locations in the code)
The variables id and its inverse invid would benefit from a more descriptive name. getID() returns the node ID as an integer, and one could expect that id would in that case be the return value of getID(), which it clearly is not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants