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

Smartsim Documentation Refactor #463

Merged
merged 97 commits into from
Mar 13, 2024
Merged

Conversation

amandarichardsonn
Copy link
Contributor

This PR provides a full refactor to the SmartSim documentation on craylabs.org. This branch merges in sections: Experiment, Orchestrator, Model, Ensemble, RunSettings, BatchSettings, and SmartSim Logging.

amandarichardsonn and others added 9 commits January 5, 2024 17:53
This PR merges the Experiment.rst file into a branch that will
encapsulate the entirety of the SmartSim documentation refactor. The
Experiment.rst file offers documentation for an Experiment, a Launcher,
the Experiment entities and an example demonstrating initializing,
starting and stopping from within an experiment. The documentation is
aimed to be accessible.

[reviewed by @mellis13 @ashao @billschereriii @juliaputko ]
[committed by @amandarichardsonn ]
This PR merges the logger document to the SmartSim docs
feature branch. The logger document encompasses information
and instruction to setup and use the SS logger.

[ reviewed by @mellis13 @billschereriii @ankona @juliaputko ]
[ committed by @amandarichardsonn ]
This PR merges into the feature branch documentation for run settings and batch settings.

[ reviewed by @ankona @mellis13 @juliaputko @billschereriii ]
[ committed by @amandarichardsonn ]
This branch will merge the Orchestrator documentation
into the main SmartSim documentation branch.

[ reviewed by @mellis13 @juliaputko @ankona @al-rigazzi ]
[ committed by @amandarichardsonn ]
This PR requests the merge of the Model documentation section into the
full craylab SmartSim doc refactor branch.

[ reviewed by @mellis13 @ankona @juliaputko @al-rigazzi ]
[ committed by @amandarichardsonn ]
This Pr merges the Ensemble documentation into the feature branch.

[ reviewed by @mellis13 @ankona ]
[ committed by @amandarichardsonn ]
Copy link
Member

@ashao ashao left a comment

Choose a reason for hiding this comment

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

Really excellent job on all the new content. In addition to the in-line comments, please address the following:

  • All code snippets should be contained in their own files. The code revealed in the documentation should just be includeliteral blocks
  • For each of these examples, at the top of their section include an expandable box that shows the full source code for that example
  • Consider what information is redundant. Ensemble and Model in particular have a lot of overlapping material. Please include summaries and discuss what the difference is when interacting with an ensemble versus a model, but make use of references to reduce the amount of repeated material.

doc/batch_settings.rst Outdated Show resolved Hide resolved
doc/batch_settings.rst Outdated Show resolved Hide resolved
doc/batch_settings.rst Outdated Show resolved Hide resolved
doc/ensemble.rst Outdated Show resolved Hide resolved
doc/ensemble.rst Outdated Show resolved Hide resolved
doc/run_settings.rst Outdated Show resolved Hide resolved
doc/run_settings.rst Outdated Show resolved Hide resolved
doc/ss_logger.rst Outdated Show resolved Hide resolved
doc/experiment.rst Outdated Show resolved Hide resolved
doc/experiment.rst Show resolved Hide resolved
Copy link
Contributor

@mellis13 mellis13 left a comment

Choose a reason for hiding this comment

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

I'm still going through the key collision section, but here is some initial feedback.

I really like the use of dropdown to show code as a single block of text!

doc/model.rst Outdated Show resolved Hide resolved
doc/model.rst Outdated Show resolved Hide resolved
doc/model.rst Outdated Show resolved Hide resolved
doc/model.rst Outdated Show resolved Hide resolved
doc/model.rst Outdated Show resolved Hide resolved
doc/model.rst Outdated Show resolved Hide resolved
doc/model.rst Outdated Show resolved Hide resolved
doc/model.rst Outdated Show resolved Hide resolved
doc/model.rst Outdated Show resolved Hide resolved
doc/model.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@mellis13 mellis13 left a comment

Choose a reason for hiding this comment

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

Review feedback for experiment.rst. Overall great improvement to Experiment API description.

doc/model.rst Outdated Show resolved Hide resolved
doc/model.rst Outdated Show resolved Hide resolved
doc/model.rst Outdated Show resolved Hide resolved
doc/experiment.rst Outdated Show resolved Hide resolved
doc/experiment.rst Outdated Show resolved Hide resolved
doc/experiment.rst Outdated Show resolved Hide resolved
doc/experiment.rst Outdated Show resolved Hide resolved
doc/experiment.rst Outdated Show resolved Hide resolved
doc/experiment.rst Outdated Show resolved Hide resolved
doc/experiment.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@mellis13 mellis13 left a comment

Choose a reason for hiding this comment

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

Very good on RunSettings. Only one minor comment.

doc/run_settings.rst Outdated Show resolved Hide resolved
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.75%. Comparing base (33ee012) to head (5d7b4a4).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #463   +/-   ##
========================================
  Coverage    90.75%   90.75%           
========================================
  Files           60       60           
  Lines         3839     3839           
========================================
  Hits          3484     3484           
  Misses         355      355           
Files Coverage Δ
smartsim/database/orchestrator.py 86.85% <ø> (ø)
smartsim/experiment.py 85.71% <ø> (ø)

@al-rigazzi al-rigazzi self-requested a review March 11, 2024 16:45
@amandarichardsonn amandarichardsonn dismissed ashao’s stale review March 13, 2024 18:01

dismissing so that I may merge

@amandarichardsonn amandarichardsonn merged commit 3c271f3 into develop Mar 13, 2024
42 checks passed
@amandarichardsonn amandarichardsonn deleted the smartsim-docs-refresh branch March 13, 2024 20:23
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