-
Notifications
You must be signed in to change notification settings - Fork 6
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
DLRM model checksum + corrected pre download clean #125
DLRM model checksum + corrected pre download clean #125
Conversation
script/download-file/run.sh
Outdated
echo ${CM_PRE_DOWNLOAD_CLEAN_CMD} | ||
${CM_PRE_DOWNLOAD_CLEAN_CMD} | ||
fi | ||
echo ${CM_PRE_DOWNLOAD_CLEAN_CMD} |
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.
Why this change is needed?
script/download-file/run.sh
Outdated
@@ -26,6 +26,9 @@ elif [ -e "${CM_DOWNLOAD_DOWNLOADED_PATH}" ]; then | |||
# checksum not supposed to fail for locally given file | |||
if [[ "${CM_DOWNLOAD_LOCAL_FILE_PATH}" != "" ]]; then | |||
exit 1 | |||
else | |||
echo "Checksum mismatch. Deleting through command: ${CM_PRE_DOWNLOAD_CLEAN_CMD}" |
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.
CM_PRE_DOWNLOAD_CLEAN=1
Is a cleaner code right?
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.
Hi @arjunsuresh , its assigned as true
in commit b40ea46 to make it uniform since in customize.py
, it expects true or false.
script/download-file/run.sh
Outdated
@@ -35,7 +37,7 @@ fi | |||
|
|||
if [[ ${require_download} == "1" ]]; then | |||
echo "" | |||
if [ -e "${CM_PRE_DOWNLOAD_CLEAN}" ]; then | |||
if [ "${CM_PRE_DOWNLOAD_CLEAN}" != "" ]; then |
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.
We should check for not equal to "False" here
This PR makes progress to issue mlcommons#167.
Other modifications:
CM_PRE_DOWNLOAD_CLEAN
env variable by default and we need to delete already existing incomplete download file.true
. The incomplete downloads will not be cleared when user explicitly setsCM_PRE_DOWNLOAD_CLEAN
tofalse