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

fix: Fix scalar model generation for L0_scalar_io #7920

Merged
merged 4 commits into from
Jan 7, 2025

Conversation

mc-nv
Copy link
Contributor

@mc-nv mc-nv commented Jan 7, 2025

This PR is fixing missed models generation.

@mc-nv mc-nv requested review from rmccorm4 and pvijayakrish January 7, 2025 21:18
@mc-nv mc-nv self-assigned this Jan 7, 2025
@mc-nv mc-nv marked this pull request as ready for review January 7, 2025 21:18
@rmccorm4 rmccorm4 changed the title Update variable name fix: Fix scalar model generation for L0_scalar_io Jan 7, 2025
@@ -286,7 +286,7 @@ python3 $VOLUME_SRCDIR/gen_qa_dyna_sequence_implicit_models.py --onnx --onnx_ops
chmod -R 777 $VOLUME_DYNASEQIMPLICITDESTDIR
python3 $VOLUME_SRCDIR/gen_qa_ragged_models.py --onnx --onnx_opset=$ONNX_OPSET --models_dir=$VOLUME_RAGGEDDESTDIR
chmod -R 777 $VOLUME_RAGGEDDESTDIR
python3 $VOLUME_SRCDIR/gen_qa_ort_scalar_models.py --onnx_opset=$ONNX_OPSET --models_dir=$SCALARMODELSDESTDIR
python3 $VOLUME_SRCDIR/gen_qa_ort_scalar_models.py --onnx_opset=$ONNX_OPSET --models_dir=$VOLUME_SCALARMODELSDESTDIR
chmod -R 777 $VOLUME_RAGGEDDESTDIR
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the next chmod line 290 also be updated?

chmod -R 777 $VOLUME_SCALARMODELSDESTDIR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For consistency probably you are right.

@mc-nv mc-nv requested a review from rmccorm4 January 7, 2025 21:32
@mc-nv mc-nv added the PR: fix A bug fix label Jan 7, 2025
rmccorm4
rmccorm4 previously approved these changes Jan 7, 2025
Copy link
Contributor

@rmccorm4 rmccorm4 left a comment

Choose a reason for hiding this comment

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

It would be nice to update the scripts to catch these types of errors earlier with something like:

if not FLAGS.models_dir:
    raise ValueError("--models_dir must be provided")

But it's fine to just do the fix if there's no bandwidth right now.


Thanks for fixing this @mc-nv !

@mc-nv mc-nv added models: fix Map models related fixes and removed PR: fix A bug fix labels Jan 7, 2025
@mc-nv mc-nv merged commit d45da99 into main Jan 7, 2025
3 checks passed
@mc-nv mc-nv deleted the mchornyi/models-typo-fix branch January 7, 2025 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
models: fix Map models related fixes
Development

Successfully merging this pull request may close these issues.

2 participants