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

Port Maestro documentation to mkdocs #403

Merged
merged 75 commits into from
Apr 28, 2023

Conversation

FrankD412
Copy link
Member

This is a running PR to port over documentation to mkdocs and the Material theme.

@jwhite242
Copy link
Collaborator

Just adding the link here for the live preview of this PR while we build this up: https://maestrowf--403.org.readthedocs.build/en/403/

@jwhite242 jwhite242 changed the title WIP: Port Maestro documentation to mkdocs Port Maestro documentation to mkdocs Feb 8, 2023
@jwhite242
Copy link
Collaborator

Alright, WIP status removed, time for reviewing this beast

@doutriaux1
Copy link
Collaborator

Front page: reproducability -> reproducibility

Getting Started:

  • should we add pip install maestrowf or at least a link to install instructions?
  • I would put a couple of line to describe what the study the user is copy/pasting is about (before saying copy/paste this)

Why Maestro:

  • I would put something similar at the top I'm not sure the general description before getting started is enough.
  • Why Meastro was created: [stub] -> needs content

Maestro page:

  • None. Set up in 5 minutes. (What is the none about?)

https://maestrowf--403.org.readthedocs.build/en/403/Philosophy/representation.html

  • graph TD; A(Setup inputs)-->B(Simulate); B-->C(Post Process); -> That seems like an actual graph is intended here

https://maestrowf--403.org.readthedocs.build/en/403/Philosophy/principles.html

  • At the end it is not clear when clicking on "next" that we move to the "User's Guide". Adding a sentence at the end of each section about what's coming up next would be great.

https://maestrowf--403.org.readthedocs.build/en/403/Maestro/index.html

  • "what is maestro": I would duplicate this or put something similar at the top of "home"
  • put a link to this section in the "quick start" in "home"
  • I would just put the "say-hello" in the home quick start, I feel the current is too complex and daunting for people coming in.

https://maestrowf--403.org.readthedocs.build/en/403/Maestro/tutorials.html

  • "These options will be covered later in these tutorials and the how-to sections ." The space after sections leads to a new line with a single dot on it. It's a bit odd.
  • "Two parameters are added, each with multiple values: NAME and GREETING. " I would go with: "Two parameters are added: NAME and GREETING, each with multiple values."

https://maestrowf--403.org.readthedocs.build/en/403/Maestro/specification.html

  • Parameter token section: "The big different with" -> "The big difference with"
  • "labels" the example: env:
    labels:
    outfile: $(SIZE.label).$(ITERATIONS.label).log
    where to SIZE and ITERATION come from? The env/variable section, the global.parameters section? Maybe we need to expand the example here even though SIZe and ITERATION are indeed defined and expalined later.
  • dependencies: can the git tag be a commit's SHA?
    *batch block: is host really required? I thought it was ignored
  • batch: add an example for lsf? there are only slurm and flux.

https://maestrowf--403.org.readthedocs.build/en/403/Maestro/scheduling.html

  • LOCAL section as [stub] only.

https://maestrowf--403.org.readthedocs.build/en/403/Maestro/parameter_specification.html

  • first 2 "warn" are not completed
  • ":ref:pgen_section is recommended." looks like :ref: is left over non parsed code
  • same in "pgen arguments" section a few :ref: there
  • "Referencing Values from a Specification's Env Block" lots of paraguayflags? I'm not sure they're intended.
  • class and func in the same function seem to be badly expanded as well.

https://maestrowf--403.org.readthedocs.build/en/403/Maestro/how_to_guides/timeouts.html

  • is --rlimit=3 the only way to control the number of restarts? Should it be in the step specs?

https://maestrowf--403.org.readthedocs.build/en/403/Maestro/how_to_guides/parameter_batches.html

  • in next steps section point to maestro musicsheets?

https://maestrowf--403.org.readthedocs.build/en/403/Maestro/reference_guide/design_reference/index.html

  • seems empty

API: I didn't look at it.

@jwhite242
Copy link
Collaborator

Alright, first pass at addressing review comments (thanks @doutriaux1 !). And one followup there to the question on host in the batch block that's not really addressed in the docs: while it is ignored by the scheduler in come cases, maestro's script adapters currently expect it to be there and will fail on a key error when trying to pop it off. It looks like there's no schema validation on these yet, so might be an opportunity for improvement there in a follow on pr to get early failures and better warning messages for the users.

highlighting ease of adding parameters and expanding that topology
once in maestro
maestro and how it helps to think about things
@jwhite242 jwhite242 force-pushed the frankd412/documentation/mkdocs branch from ef3f96f to 5753b6e Compare April 28, 2023 00:43
@jwhite242 jwhite242 merged commit 78cc329 into LLNL:develop Apr 28, 2023
jwhite242 added a commit that referenced this pull request Dec 12, 2023
1.1.10 Release (#432)

* Sync up read the docs config with dev environments using poetry (#399)
* Print usage on command line when no args are provided (#404)
* Add sacct fallback to slurm adapter to improve robustness of job tracking (#405)
* Update Flurm Job State mappings for flux versions >= 0.26 (#407)
* Bump certifi from 2021.10.8 to 2022.12.7 to address security issue (#409)
* Bump cryptography from 37.0.1 to 38.0.3 to address security issue (#410)
* Add missing shbang in unscheduled scripts from lsf adapter (#411)
* Update poetry lockfile to address dependabot flagged security issues (#412)
* Fix for Dockerfile smell DL3006 (#418)
* Port Maestro documentation to mkdocs and expand coverage of features and tutorials (#403)
* Update version info to be driven from pyproject.toml exclusively, and hook up to command line (#419)
* Pin mermaid to < 10.x due to api change (#422)
* Bump lock file certifi from 2022.12.7 to 2023.7.22 to address security issue (#426)
* Refactor flux adapter to avoid using pickle to talk to flux brokers installed in external environments (#415)
   Also adds flux integration tests to exercise against real flux brokers
* Add pager functionality to status command (#420)
* Patch broken flux job cancellation (#428)
* Insulate slurm adapters from user customization of squeue and sacct output formats (#431)
   Also adds live unit and integration tests for slurm adapter

---------

Co-authored-by: Francesco Di Natale <frank.dinatale1988@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Bruno P. Kinoshita <kinow@users.noreply.github.com>
Co-authored-by: Charles Doutriaux <doutriaux1@llnl.gov>
Co-authored-by: Giovanni Rosa <grosa23@yahoo.com>
Co-authored-by: Brian Gunnarson <49216024+bgunnar5@users.noreply.github.com>
jwhite242 added a commit that referenced this pull request Feb 6, 2024
1.1.10 Release (#432)

* Sync up read the docs config with dev environments using poetry (#399)
* Print usage on command line when no args are provided (#404)
* Add sacct fallback to slurm adapter to improve robustness of job tracking (#405)
* Update Flurm Job State mappings for flux versions >= 0.26 (#407)
* Bump certifi from 2021.10.8 to 2022.12.7 to address security issue (#409)
* Bump cryptography from 37.0.1 to 38.0.3 to address security issue (#410)
* Add missing shbang in unscheduled scripts from lsf adapter (#411)
* Update poetry lockfile to address dependabot flagged security issues (#412)
* Fix for Dockerfile smell DL3006 (#418)
* Port Maestro documentation to mkdocs and expand coverage of features and tutorials (#403)
* Update version info to be driven from pyproject.toml exclusively, and hook up to command line (#419)
* Pin mermaid to < 10.x due to api change (#422)
* Bump lock file certifi from 2022.12.7 to 2023.7.22 to address security issue (#426)
* Refactor flux adapter to avoid using pickle to talk to flux brokers installed in external environments (#415)
   Also adds flux integration tests to exercise against real flux brokers
* Add pager functionality to status command (#420)
* Patch broken flux job cancellation (#428)
* Insulate slurm adapters from user customization of squeue and sacct output formats (#431)
   Also adds live unit and integration tests for slurm adapter

---------

Co-authored-by: Francesco Di Natale <frank.dinatale1988@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Bruno P. Kinoshita <kinow@users.noreply.github.com>
Co-authored-by: Charles Doutriaux <doutriaux1@llnl.gov>
Co-authored-by: Giovanni Rosa <grosa23@yahoo.com>
Co-authored-by: Brian Gunnarson <49216024+bgunnar5@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants