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

update vignette for efficient calculations #598

Merged
merged 1 commit into from
Aug 30, 2021

Conversation

PavelBal
Copy link
Member

@msevestre Nice description, indeed! I corrected some typos and added some explanation to the vignette, let me know what do you think.



## Running one or more simulations multiple times in a loop
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the feeling that this was not the right caption? I did not see a section where you describe a loop. I propose a new caption.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would only use batch if it was in a loop such as an algorithm (PI, sensitivity etc..) .For a finite number of run, I am not sure that this is really required.....

library(ospsuite)
simFilePath <- system.file("extdata", "Aciclovir.pkml", package = "ospsuite")

# We create two different instances of the same simulation to illustrate the usage.
# We load the simulation for which the batches will be created
sim1 <- loadSimulation(simFilePath, loadFromCache = FALSE)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, no two sims needed here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this was just to say that you can have multiple simulations and not only one simulation

parametersOrPaths = c("Aciclovir|Molecular weight")
)

simulationBatches <- list(simBatch1, simBatch2)
```

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split the code into smaller chunks with some info in between.


# So far, we created 2 simulation batches, one with 3 parameter sets and the
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it a bit too technical, with memory allocation etc..

simulationResults <- runSimulations(simulations = sim)
print(simulationResults)
The results in this case will be a named list of `SimulationResults`-object , i.e. one for each simulation.
The names of the entries of the list are the ids of the simulation to which the results
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some info about ids.

@codecov-commenter
Copy link

Codecov Report

Merging #598 (f0ec730) into develop (be7c1ae) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #598   +/-   ##
========================================
  Coverage    90.27%   90.27%           
========================================
  Files           75       75           
  Lines         1831     1831           
========================================
  Hits          1653     1653           
  Misses         178      178           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be7c1ae...f0ec730. Read the comment docs.

# from the environment and the allocated memory cleared.
# This will be done automatically when the R session is terminated.
rm(simBatch1)
rm(simBatch2)
```

Usage of `SimulationBatch` is recommended for advanced scenarios where simulations expected to be run hundreds of times and where each second that can be spared will impact the performance significantly.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not do this as it is MD and therefor will be formatted automatically. Why do you think we need to add CR manually?

@msevestre
Copy link
Member

Fine with me

@msevestre msevestre merged commit a5bbb49 into develop Aug 30, 2021
@msevestre msevestre deleted the efficient-calculations-vignette branch August 30, 2021 14:13
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