Addition of a LULESH study specification with heavier comments. by FrankD412 · Pull Request #132 · llnl/maestrowf

FrankD412 · 2018-08-08T20:16:40Z

No description provided.

FrankD412 · 2018-08-09T00:32:04Z

@jsemler @kcathey @tadesautels -- feel free to comment as I commit. I'm trying to make comments as clear as possible.

jsemler

A couple suggestions, but overall looks good.

jsemler · 2018-08-09T02:52:55Z

+#   description - Description of what this study does.
+#-------------------------------
+# NOTE: You can add other keys to this block for custom documentation. Maestro
+# currently on looks for the required set.


Should this be: "currently only looks for the required set"?

Good catch. Fixed.

jsemler · 2018-08-09T03:22:04Z

+          depends: []
+
+      # 'run-lulesh' is an example of a parameterized step because the task
+      # contains parameters (more on those later).


Consider clarifying that the parameters being referenced in the step are $(SIZE) and $(ITERATIONS).

Good point -- added.

jsemler · 2018-08-09T19:41:32Z

+####################################
+# The parameter block contains all the things we'd like to vary in the study.
+# Currently, there are two modes of operating in the specification:
+# 1. If a parameter block is specified, the study is expanded and considered a


Does the parameter also have to be referenced in a step?

I added a statement to this that a parameter does not necessarily need to be used.

Is the question if global.parameters implies the non-Linear expansion, even if the parameters are unused?

jsemler · 2018-08-09T19:52:24Z

+# 2. A label that represents a "pretty printed" version of the parameter. The
+#    parameter values is specfied by the '%%' moniker (for example, for SIZE --
+#    when SIZE is equal to 10, the label will be 'SIZE.10').
+# 3. (Optional) A list of custom names for each value in the values list.


If names is specified is that used instead of the parameter values when creating the workspace directories?

This comment prompted me to go back and realize I was not remembering the functionality correctly. This'll be fixed in the next update.

tadesautels · 2018-08-09T19:54:47Z

+####################################
+# The description block is where the description of the study is placed. This
+# section is meant primarily for documentation purposes so that when a
+# spcecifcation is passed to other users they can gleam a general understanding


"gleam" --> "glean"

tadesautels · 2018-08-09T19:57:29Z

+    # concept to Unix environment variables. These variables are not dependent
+    # on values in the environment and so are more portable.
+
+    # NOTE: These values are substituted as static strings meaning that other


Drop first "other"

tadesautels · 2018-08-09T20:00:09Z

+        # OUTPUT_PATH is a keyword variable that Maestro looks for in order to
+        # set a custom output path for the study workspace. If not specified,
+        # OUTPUT_PATH is assumed to be the path where Maestro was launched from.
+        OUTPUT_PATH: ./sample_output/lulesh


I have found this one a bit confusing; it's really more of a workspace_root, rather than an output_path.
Renaming this variable would probably be best, but the description should certainly say that this is where ALL of maestro's temporary files will go, in addition to any output, and typically within a custom-named, date-stamped directory for this run of maestro.

I corrected this in my last commit -- let me know if it's still unclear.

tadesautels · 2018-08-09T20:04:14Z

+          # NOTE: There are additional step entries for setting up a step for
+          # remote execution. For examples see 'launcher_tokens.yaml' in the
+          # 'samples/documentation' subdirectory in the maestrowf repo.
+


I presume you're going to eventually have a more complete, formal list of all of the options, right?

Yeah, that the eventual goal. The comments here are meant to help users make sense of this particular spec and to get the generals in place.

tadesautels · 2018-08-09T20:12:04Z

+      # to have a step that is expanded because its dependencies have been
+      # expanded. For example, if you wanted to run individual post processing
+      # after each 'run-lulesh' step, you might have a step called
+      # 'post-process-run' that is dependent on 'run-lulesh'. This notation


This should explicitly say, "the job step specification would then contain the value 'depends : [run_lulesh]'."

If this is what you mean.

kcathey · 2018-08-09T20:28:18Z

+            sed -i 's/^CXXFLAGS = -g -O3 -fopenmp/#CXXFLAGS = -g -O3 -fopenmp/' ./Makefile
+            sed -i 's/^#LDFLAGS = -g -O3/LDFLAGS = -g -O3/' ./Makefile
+            sed -i 's/^LDFLAGS = -g -O3 -fopenmp/#LDFLAGS = -g -O3 -fopenmp/' ./Makefile
+            sed -i 's/^#CXXFLAGS = -g -O3 -I/CXXFLAGS = -g -O3 -I/' ./Makefile


I know that this out of our control, but these flags should really be configured using a configure script that lulesh supplies. We might want to send that back to the lulesh folks. Maestro would then not need to know the defaults of how to build lulesh, just run configure make clean and make.

That's a fine point. I agree. At the very least maybe a make serial or make parallel.

* Updates to the README. * Added initial getting started example documentation * Addition of Quick Start documentation. (#130) * Addition of Quick Start. * Addition of cancellation status example. * Corrections to quick start and fix for build errors. * Addition of a LULESH study specification with heavier comments. (#132) * Start of a LULESH spec that's commented. * Addition of explanation of step structure. * Additional comments about steps and dependencies. * Minor tweaks to comments. * Addition of parameter block comments. * Addition of reference to launcher_token YAML * Tweaks and edits based on feedback.

* Update setup.py to 1.1.3dev * Updated the getting started documentation * Added examples for running both lulesh specs * Flux for Spectrum bugfixes (#116) * Additional arguments can be passed through batch. * Correction of the use of extend to append. * Removal of OMPI vars from env. * Reversal of env altering. * Addition of mpi exe to batch. * Removal of -gpu flag * Addition of "allocated" to PENDING set. * Correction of the check_status method call. * Correction of the Flux import * Addition of the EnvironmentError to try/catch for check_status. * Addition of jobid to submission INFO logging. * Workflow setup fix (#117) * Fixed a workflow setup issue caused by spaces in the spec name * Additional dirname formatting * Refactor of the Study class to breakdown complex APIs (#118) * Minor docstring correction. * Change to study setup API to break out workspace creation. * Update Maestro frontend to use new Study method. * Renamed Study.setup to be better communicate its functionality. * Correction to style for configure_study method. * Moved environment application to add_step. * Split out acquiring environment elements to its own method. * Updates to staging checks. * Renamed _setup_linear and _setup_parameterized from setup to stage for clarity. * Tweaks and addition of starting to SpectrumFluxAdapter. (#119) * Updated the LULESH examples to use the LULESH git repository for cloning the repo (#121) * Add the generation of metadata to Study construction. (#120) * Addition of method and tweaks to support metadata. * Addition of call to the method for generating metadata. * Correction to encode YAML string for py3 compatibility. * Another attempt at py2-py3 compatible encoding. * Addition of load_metadata method. * Additional debug logging for the Flux Spectrum Adapter (#122) * Additional logging for debugging of the adapter. * Additional logging. * Updated Exception.message references to Exception.args (#125) * Fix status.csv 'State' column writeout. (#127) * #107 Enhance/return codes (#128) * #107 Added StudyStatus enumeration Changed DAG execute_ready_steps to return StudyStatus enum instead of a boolean conductor and maestro also accept StudyStatus and convert to return value * #128 fixed issues found in pull request * #107 added inline comments per PC * #107 pulling out _check_study_completion to avoid initial implementaion copy and paste error * #107 fixed line length issue * Bugfix that would cause the ExecutionGraph to not update when cancelled. (#131) * Addition of custom parameter generation when running studies. (#129) * Command line parameter for custom ParameterGenerators. * Addition of sample custom parameter generation. * Correction to missing parameters in SIZE. * Addition of utility method for importing custom gen py files. * Correction of syntax. * Conversion of DOS newlines to Unix. * Addition of call to check for custom code. * Correction of a bad docstring. * Addition of parameter metadata to Maestro. * Addition of parameters to study metadata. * Addition of copying of the parameter file to study workspace. * Correction to a bug for attempting a restart when a restart isn't specified. (#133) * Initial fix for steps that cannot restart. * Comments to clarify logic. * More flexible ExecutionGraph description API and logging of description. (#137) * Tweaks to how the ExecutionGraph adds descriptions. Added logging. * Updated study to use the new API and some minor refactor. * Fixed #134 * Fix to Variable verification. (#138) Fixes #123 * Updates to Maestro documentation (#114) * Updates to the README. * Added initial getting started example documentation * Addition of Quick Start documentation. (#130) * Addition of Quick Start. * Addition of cancellation status example. * Corrections to quick start and fix for build errors. * Addition of a LULESH study specification with heavier comments. (#132) * Start of a LULESH spec that's commented. * Addition of explanation of step structure. * Additional comments about steps and dependencies. * Minor tweaks to comments. * Addition of parameter block comments. * Addition of reference to launcher_token YAML * Tweaks and edits based on feedback. * Throttled workflows push steps into the ready queue multiple times. (#136) * Moved 'mark_submitted' into ExecutionGraph 'execute' * Conversion of ready_steps deque to use step names. * Capping the available slots to the length of ready_steps. * Fixes Popen output to use universal newlines. (#143) * Addition of a shell util to centralize process creation settings. * Switch to start_process method. * Addition of docstring for start_process. * Update to fix universal newline from false to true. * Addition of a check for a list and a hard set of shell to False. * Made conductor launch command a string. * Fix behavior of start_process for localscriptadapter. The shell=False flag was not being passed, nor were the env or cwd arguments being used in the actual function. * Clean up of start_process * Cleans up GitDependency errors to not rely on return codes. (#144) * Addition of a ping method. * Tweaks to error logging. * Moved specific existence exception out of return code check. * Cleanup of some merge characters. * Tweak for connectivity checking. * Addition of info to the error message. * Removal of git return codes since they don't mean anything. * Update version number to 1.1.3.

* Updates to the README. * Added initial getting started example documentation * Addition of Quick Start documentation. (#130) * Addition of Quick Start. * Addition of cancellation status example. * Corrections to quick start and fix for build errors. * Addition of a LULESH study specification with heavier comments. (#132) * Start of a LULESH spec that's commented. * Addition of explanation of step structure. * Additional comments about steps and dependencies. * Minor tweaks to comments. * Addition of parameter block comments. * Addition of reference to launcher_token YAML * Tweaks and edits based on feedback.

Start of a LULESH spec that's commented.

475bca0

FrankD412 added In Progress Issue or PR that is currently in active development. Documentation labels Aug 8, 2018

FrankD412 self-assigned this Aug 8, 2018

FrankD412 requested review from jsemler, kcathey and tadesautels August 8, 2018 20:16

FrankD412 changed the base branch from develop to documentation_update August 8, 2018 20:17

FrankD412 added 2 commits August 8, 2018 15:47

Addition of explanation of step structure.

5e11b7e

Additional comments about steps and dependencies.

5e40b20

jsemler reviewed Aug 9, 2018

View reviewed changes

FrankD412 added 3 commits August 8, 2018 21:08

Minor tweaks to comments.

48d455a

Addition of parameter block comments.

fe59069

Addition of reference to launcher_token YAML

c6e8fc5

jsemler reviewed Aug 9, 2018

View reviewed changes

tadesautels reviewed Aug 9, 2018

View reviewed changes

kcathey reviewed Aug 9, 2018

View reviewed changes

kcathey approved these changes Aug 9, 2018

View reviewed changes

Tweaks and edits based on feedback.

39bf579

jsemler approved these changes Aug 10, 2018

View reviewed changes

FrankD412 merged commit 09a27d4 into documentation_update Aug 13, 2018

FrankD412 deleted the documentation/lulesh_comments branch August 13, 2018 18:01

FrankD412 removed the In Progress Issue or PR that is currently in active development. label Aug 17, 2018

Conversation

FrankD412 commented Aug 8, 2018

Uh oh!

FrankD412 commented Aug 9, 2018

Uh oh!

jsemler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants