Optimize worker behavior for object store writes #650
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,54 +4,65 @@ | |
| while [ ${#} -gt 0 ]; do | ||
| case "${1}" in | ||
| --config-dataset) | ||
| declare -x CONFIG_DATASET_NAME="${2:?}" | ||
| shift | ||
| ;; | ||
| --host-string) | ||
| declare -x MPI_HOST_STRING="${2:?}" | ||
| shift | ||
| ;; | ||
| --hydrofabric-dataset) | ||
| declare -x HYDROFABRIC_DATASET_NAME="${2:?}" | ||
| shift | ||
| ;; | ||
| --job-id) | ||
| declare -x JOB_ID="${2:?}" | ||
| shift | ||
| ;; | ||
| --node-count) | ||
| declare -x MPI_NODE_COUNT="${2:?}" | ||
| shift | ||
| ;; | ||
| --output-dataset) | ||
| declare -x OUTPUT_DATASET_NAME="${2:?}" | ||
| shift | ||
| ;; | ||
| --partition-dataset) | ||
| declare -x PARTITION_DATASET_NAME="${2:?}" | ||
| shift | ||
| ;; | ||
| --worker-index) | ||
| declare -x WORKER_INDEX="${2:?}" | ||
| shift | ||
| ;; | ||
| --calibration-config-file) | ||
| declare -x CALIBRATION_CONFIG_BASENAME="${2:?}" | ||
| shift | ||
| ;; | ||
| esac | ||
| shift | ||
| done | ||
|
|
||
| # TODO: (later) in both ngen and ngen-cal entrypoints, add controls for whether this is temp dir or output dataset dir | ||
| declare -x JOB_OUTPUT_WRITE_DIR="/tmp/job_output" | ||
|
|
||
| # Get some universally applicable functions and constants | ||
| source ./funcs.sh | ||
|
|
||
| ngen_sanity_checks_and_derived_init | ||
| init_script_mpi_vars | ||
| init_ngen_executable_paths | ||
|
|
||
| # Move to the output write directory | ||
| # TODO: (later) in both ngen and ngen-cal entrypoints, control whether this is needed, based on if write dir is output dataset dir | ||
| #cd ${OUTPUT_DATASET_DIR:?Output dataset directory not defined} | ||
| mkdir ${JOB_OUTPUT_WRITE_DIR:?} | ||
| chown ${MPI_USER}:${MPI_USER} ${JOB_OUTPUT_WRITE_DIR} | ||
| cd ${JOB_OUTPUT_WRITE_DIR} | ||
| #Needed for routing | ||
| if [ ! -e /dmod/datasets/linked_job_output ]; then | ||
| ln -s ${JOB_OUTPUT_WRITE_DIR} /dmod/datasets/linked_job_output | ||
| fi | ||
|
|
||
| start_calibration() { | ||
| # Start ngen calibration | ||
|
|
@@ -61,24 +72,31 @@ start_calibration() { | |
| echo "$(print_date) Starting ngen calibration with serial ngen execution" | ||
| fi | ||
|
|
||
| # Find calibration config, then copy to output dataset and use that | ||
| if [ -n "${CALIBRATION_CONFIG_BASENAME:-}" ]; then | ||
| #CALIBRATION_CONFIG_FILE=$(find ${OUTPUT_DATASET_DIR:?} -type f -name "${CALIBRATION_CONFIG_BASENAME}" -maxdepth 1 | head -1) | ||
| _ORIG_CAL_CONFIG_FILE=$(find ${CONFIG_DATASET_DIR:?} -type f -name "${CALIBRATION_CONFIG_BASENAME}" -maxdepth 1 | head -1) | ||
| else | ||
| #CALIBRATION_CONFIG_FILE=$(find ${OUTPUT_DATASET_DIR:?} -type f -iname "*.yaml" -o -iname "*.yml" -maxdepth 1 | head -1) | ||
| _ORIG_CAL_CONFIG_FILE=$(find ${CONFIG_DATASET_DIR:?} -type f -iname "*.yaml" -o -iname "*.yml" -maxdepth 1 | head -1) | ||
| fi | ||
| if [ -z "${_ORIG_CAL_CONFIG_FILE}" ]; then | ||
| echo "Error: NGEN calibration yaml file not found" 2>&1 | ||
| exit 1 | ||
| fi | ||
| cp -a ${_ORIG_CAL_CONFIG_FILE:?} ${OUTPUT_DATASET_DIR:?}/. | ||
|
There was a problem hiding this comment. Marking this as related to #407. I am not sure where we should document the expectations / spec for a worker image's data requirements, but this feels like a prime candidate for that sort of documentation. Just want to track this b.c. it will affect how we need to normalize paths. |
||
| CALIBRATION_CONFIG_FILE="${OUTPUT_DATASET_DIR:?}/$(basename ${_ORIG_CAL_CONFIG_FILE})" | ||
|
|
||
| python3 -m ngen.cal "${CALIBRATION_CONFIG_FILE}" | ||
|
|
||
| #Capture the return value to use as service exit code | ||
| NGEN_RETURN=$? | ||
|
|
||
| echo "$(print_date) ngen calibration finished with return value: ${NGEN_RETURN}" | ||
|
|
||
| # TODO: (later) make sure outputs are handled properly, and that eventually we support toggling whether written to | ||
| # TODO: output dataset dir directly or somewhere else | ||
|
|
||
| # Exit with the model's exit code | ||
| return ${NGEN_RETURN} | ||
| } | ||
|
|
@@ -89,11 +107,16 @@ if [ "${WORKER_INDEX:-0}" = "0" ]; then | |
| # This will only have an effect when running with multiple MPI nodes, so its safe to have even in serial exec | ||
| trap close_remote_workers EXIT | ||
| # Have "main" (potentially only) worker copy config files to output dataset for record keeping | ||
| # TODO: (later) in ngen and ngen-cal entrypoints, consider adding controls for whether this is done or a simpler | ||
| # TODO: 'cp' call, based on whether we write directly to output dataset dir or some other output write dir | ||
| # Do a dry run first to sanity check directory and fail if needed before backgrounding process | ||
| py_funcs tar_and_copy --dry-run --compress ${CONFIG_DATASET_DIR:?Config dataset directory not defined} config_dataset.tgz ${OUTPUT_DATASET_DIR:?} | ||
| # Then actually run the archive and copy function in the background | ||
| py_funcs tar_and_copy --compress ${CONFIG_DATASET_DIR:?} config_dataset.tgz ${OUTPUT_DATASET_DIR:?} & | ||
| _CONFIG_COPY_PROC=$! | ||
|
There was a problem hiding this comment. Shouldn't we There was a problem hiding this comment. I am mostly kicking that can down the road, but practically I don't think we will have to. But this image is going to need extensive test regardless, and we can sort out any issues related to this then. |
||
| # If there is partitioning, which implies multi-processing job ... | ||
| if [ -n "${PARTITION_DATASET_DIR:-}" ]; then | ||
| # Include partition config dataset too if appropriate | ||
| cp -a ${PARTITION_DATASET_DIR}/. ${OUTPUT_DATASET_DIR:?} | ||
| fi | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to @christophertubbs's above comment about using python for example. While out of scope for this PR, we should really consider rewriting this entry point and related helper functions in something other than POSIX compliant shell.