Skip to content

Improve MPI-related and Simulation docs#110

Merged
ziotom78 merged 14 commits intomasterfrom
fix109
Sep 22, 2021
Merged

Improve MPI-related and Simulation docs#110
ziotom78 merged 14 commits intomasterfrom
fix109

Conversation

@ziotom78
Copy link
Copy Markdown
Member

@ziotom78 ziotom78 commented Apr 7, 2021

This PR adds the following features to the documentation:

  • It corrects the syntax used to turn MPI on/off in a Simulation object (we do no longer use the use_mpi flag);
  • It adds a (long) docstring to Simulation.create_observations;
  • It removes references to use_mpi that were still present in docstrings;
  • It fixes a few minor errors in the Sphinx documentation.

@ziotom78
Copy link
Copy Markdown
Member Author

ziotom78 commented Apr 7, 2021

This should fix issue #109. Note that I decided not to remove distribute, as I realized that there are significant use cases where it might be indispensable (e.g., when you do not want to rely on the standard way of distributing observations offered by the framework).

@ziotom78 ziotom78 requested review from dpole and paganol April 7, 2021 13:04
@dpole
Copy link
Copy Markdown
Member

dpole commented Apr 13, 2021

Thanks Maurizio, I am still a bit confused by all the possible interactions that can happen between the parameters num_of_obs_per_detector, distribute and n_blocks_time.

num_of_obs_per_detector chunks the time and keep the chunks as a list of observation. How the content in the memory of the process changes depending on the values of distribute or Simulation.mpi_comm?

@ziotom78
Copy link
Copy Markdown
Member Author

I have renamed the confusingly-named distribute parameter to split_list_over_processes and have added a (quite long) docstring to Simulation.create_observations. @dpole , @paganol , this should answer the perplexities we had, what do you think?

@ziotom78 ziotom78 merged commit c5c01c6 into master Sep 22, 2021
@ziotom78 ziotom78 deleted the fix109 branch September 22, 2021 07:12
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