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

[LIBSTELL] Use a more direct (i.e. no loops) implementation of mpi_calc_myrange #244

Merged
merged 6 commits into from
Jun 4, 2024

Conversation

jonathanschilling
Copy link
Collaborator

This is a further iteration of #240.

This is a fundamental and substantial change to a piece of code used by many routines.

I agree and therefore, I would say that it would be valuable to have this routine very well documented and almost obvious to understand.
In the suggested change, the assumptions and the approach are (I hope) quite explicitly documented and reasoned about. Please let me know if you think differently about this.

I also attached a stand-alone test program (test_mpi_calc_myrange) now that I used to verify that the computed mystart and myend are identical (for the probed parameter value ranges) to what the old implementation produced:
test_mpi_calc_myrange.zip
It is compiled with gfortran test_mpi_calc_myrange.f -o test_mpi_calc_myrange
and then run with ./test_mpi_calc_myrange and should then print all tests passed successfully.
I understand that it was not obvious from the previous iteration of this PR that the outputs are identical. Sorry for that.

jons-pf added 2 commits June 3, 2024 16:40
… This one also checks that no errornous mystart, myend are created if more ranks are tried to be assigned than work items (e.g. loop iterations, field lines to be traced, ...) are available
@lazersos
Copy link
Collaborator

lazersos commented Jun 4, 2024

@jonathanschilling This looks better. All your types are integers and so integer math is used, this could introduce issues. I'll double check this today and get back to this.

@jonathanschilling
Copy link
Collaborator Author

jonathanschilling commented Jun 4, 2024

Thanks! The use of integer math is intended, as the remainder of the division is explicitly handled via the work_remainder variable. Do you know a way to make it more explicit in the code that integer division is intended? (I assume that you are concerned about the division in work_per_rank = total_work / local_size, right?)

EDIT: added comments in the code to preserve this discussion

@lazersos lazersos merged commit 87f189c into develop Jun 4, 2024
2 checks passed
@lazersos lazersos deleted the direct_mpi_calc_myrange branch June 4, 2024 13:59
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.

3 participants