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

Updated code with inclusive terms leader and follower #739

Merged
merged 1 commit into from
Feb 24, 2021

Conversation

mcianfrocco
Copy link
Contributor

Hello RELION developers -

Related to discussions of how to make science and cryoEM inclusive, we should consider removing 'master' and 'slave' from our code. To help facilitate this, I'm submitting this pull request where I replaced 'master' and 'slave' with 'leader' and 'follower,' respectively.

Some additional materials discussing this situation in the life sciences & computing:

I've tested the changes on my end without any issues in analyzing the tutorial dataset.

If you prefer alternative terms I can also change and submit a different pull request.

Thank you!
Mike

Screen Shot 2021-02-23 at 12 27 50 PM

@biochem-fan
Copy link
Member

Since I am not a native speaker of English, I cannot properly evaluate the connotation of these terms. I will leave decision to @scheres.

@eriklindahl
Copy link

For GROMACS we're trying to move to server/client, but the exact nomenclature isn't that critical, I think.

One can of course easily argue a particular change in a particular source code won't have huge impact in the world, and that people have many other priorities, but most of us are also privileged and have never felt the injustice of history. In particular when @mcianfrocco has already done the work I would strongly suggest we simply merge this to do our part.

@biochem-fan biochem-fan changed the base branch from master to ver3.1 February 23, 2021 22:11
@biochem-fan
Copy link
Member

I rebased the patch to ver3.1, which is the latest (public) development branch and will be 3.1.2 in near future.

@mcianfrocco
Copy link
Contributor Author

Thank you @biochem-fan & @eriklindahl !

@scheres
Copy link
Contributor

scheres commented Feb 24, 2021

Thanks for bringing this up Mike. I don't really like leader/follower. Would replacing 'slave' by 'worker', and leaving master as it is be an alternative?

@eriklindahl
Copy link

We've discussed that in other occasions (codes), and the argument there has been that when the two have occurred as a pair for 30+ years, it looks half-hearted to only replace one of them (since the word definitely has been referring to a certain type of "master"). I would suggest either server-client, server-worker, director-worker or controller-worker to just avoid that.

@biochem-fan biochem-fan reopened this Feb 24, 2021
@biochem-fan
Copy link
Member

@mcianfrocco

Sorry, I was not clear. I meant I changed the target of the patch to ver3.1 by "rebased"; in other words, the patch is not yet merged. The discussion is ongoing. I reopened this PR.

@delarosatrevin
Copy link
Member

delarosatrevin commented Feb 24, 2021 via email

@scheres
Copy link
Contributor

scheres commented Feb 24, 2021

@mcianfrocco did you need to do anything else than a sed operation for master, Master, slave & Slave on the entire code base? Any pitfalls there?

@scheres
Copy link
Contributor

scheres commented Feb 24, 2021

Hmmm, I quite like worker, but don't really like the alternatives for rank=0. Thinking about what is going on a bit more, perhaps leader/follower is actually not so bad: it is more symmetric and less emotional. Another advantage is that it keeps @mcianfrocco 's suggestion, who brought it up in the first place. Will merge now. Thanks again Mike.

@scheres scheres merged commit 7c1fc17 into 3dem:ver3.1 Feb 24, 2021
@biochem-fan
Copy link
Member

I've tested the changes on my end without any issues in analyzing the tutorial dataset.

This patch was incomplete.

diff --git a/src/acc/cuda/cuda_autopicker.cu b/src/acc/cuda/cuda_autopicker.cu
index e52309a..61467b4 100644
--- a/src/acc/cuda/cuda_autopicker.cu
+++ b/src/acc/cuda/cuda_autopicker.cu
@@ -77,7 +77,7 @@ AutoPickerCuda::AutoPickerCuda(AutoPickerMpi *basePicker, int dev_id, const char
 
 {
        node = basePicker->getNode();
-       basePicker->verb = (node->isMaster()) ? 1 : 0;
+       basePicker->verb = (node->isLeader()) ? 1 : 0;
 
        projectors.resize(basePckr->Mrefs.size());
        have_warned_batching=false;

@biochem-fan
Copy link
Member

Actually there were many remaining occurrences in the source codes. I replaced them but please make a complete pull request next time.

@mcianfrocco
Copy link
Contributor Author

Thank you @biochem-fan - I apologize for missing these instances.

One version that came from Ben Himes on Twitter was Dispatcher / Agent.

Thank you for considering & including these changes!
mike

@asntech
Copy link

asntech commented Feb 25, 2021

Thanks @mcianfrocco @biochem-fan @scheres and RELION developers/contributors for taking such quick action/decision to replace the problematic terms!! 👏🏼

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

Successfully merging this pull request may close these issues.

6 participants