Skip to content

fix: Added magpieTTS#70

Open
styagi130 wants to merge 4 commits intonvidia-riva:mainfrom
styagi130:dev/siddhartht/magpie_tts
Open

fix: Added magpieTTS#70
styagi130 wants to merge 4 commits intonvidia-riva:mainfrom
styagi130:dev/siddhartht/magpie_tts

Conversation

@styagi130
Copy link

No description provided.

Signed-off-by: Siddharth Tyagi <siddhartht@nvidia.com>
Signed-off-by: Siddharth Tyagi <siddhartht@nvidia.com>
@rmittal-github
Copy link
Contributor

@jmayank1511 to review as well

@rmittal-github
Copy link
Contributor

@styagi130 can we add magpie instructions to the README for users to refer?
https://github.com/nvidia-riva/nemo2riva/tree/main?tab=readme-ov-file#supported-models

Signed-off-by: Siddharth Tyagi <siddhartht@nvidia.com>
export_file = os.path.join(tmpdir, export_filename)

if cfg.export_format in ["ONNX", "TS"]:
if cfg.export_format in ["ONNX", "TS"] and (model.__class__.__name__ == "MagpieTTSModel" and args.submodel == "encoder"):

Choose a reason for hiding this comment

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

should this be or ?

Copy link
Author

Choose a reason for hiding this comment

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

Updated in the latest commit

Copy link
Contributor

Choose a reason for hiding this comment

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

@virajkarandikar good to resolve?

requirements.txt Outdated
nvidia-eff-tao-encryption>=0.1.8
nvidia-pyindex==1.0.6
onnx==1.18.0
onnx==1.16.0

Choose a reason for hiding this comment

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

will this break other models ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@styagi130 to update to not change onnx version requirements, so as not to disturb ASR.

Copy link
Author

Choose a reason for hiding this comment

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

Moved onnx version==1.18.0 in the latest commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

@virajkarandikar good to resolve this comment?

Copy link

@virajkarandikar virajkarandikar left a comment

Choose a reason for hiding this comment

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

added comment

@rmittal-github
Copy link
Contributor

also, @styagi130 let's rebase this

Signed-off-by: Siddharth Tyagi <siddhartht@nvidia.com>
# "path_type": "TAR_PATH",
# "content": artifact_content,
#}
#f.close()
Copy link

Choose a reason for hiding this comment

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

can we remove this code block if commented and use context manager for file operation

Copy link
Contributor

Choose a reason for hiding this comment

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

@styagi130 could you review this comment and update as needed?

@rmittal-github
Copy link
Contributor

@jmayank1511 does this look fine from ASR perspective?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants