Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions examples/tts/magpietts_inference.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,9 @@ def run_inference_and_evaluation(
violin_plot_metrics.remove('utmosv2')

# Build full checkpoint identifier (include MoE info if present)
full_checkpoint_name = f"{checkpoint_name}_{moe_info}{inference_config.build_identifier()}_SV_{eval_config.sv_model}_{eval_config.language}"
full_checkpoint_name = (
f"{checkpoint_name}_{moe_info}{inference_config.build_identifier()}_SV_{eval_config.sv_model}"
)
Comment on lines +219 to +221

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Preserve language in aggregate CSV identity.

full_checkpoint_name no longer contains the dataset language, but append_metrics_to_csv still writes it at Lines 349 and 376 while the CSV schema has no language column. Multilingual runs can therefore produce indistinguishable aggregate rows for the same checkpoint and dataset. Add a language column or pass a language-qualified checkpoint identifier to the aggregate CSV writer.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/tts/magpietts_inference.py` around lines 219 - 221, Update the
checkpoint identity flow around full_checkpoint_name and append_metrics_to_csv
so aggregate CSV rows preserve dataset language. Either add a language column to
the aggregate CSV schema and populate it at both writer call sites, or include
the language in the checkpoint identifier passed to the writer; ensure
multilingual runs remain distinguishable for the same checkpoint and dataset.


# Tracking metrics across datasets
ssim_per_dataset = []
Expand Down Expand Up @@ -261,7 +263,7 @@ def run_inference_and_evaluation(
}

# Setup output directories
eval_dir = os.path.join(out_dir, f"{full_checkpoint_name}_{dataset}")
eval_dir = os.path.join(out_dir, f"{full_checkpoint_name}_{language}_{dataset}")
audio_dir = os.path.join(eval_dir, "audio")
os.makedirs(eval_dir, exist_ok=True)

Expand Down Expand Up @@ -336,12 +338,14 @@ def run_inference_and_evaluation(
filewise_metrics_all_repeats.extend(filewise_metrics)

# Save metrics
with open(os.path.join(eval_dir, f"{dataset}_metrics_{repeat_idx}.json"), "w") as f:
metrics_path = os.path.join(eval_dir, f"{dataset}_metrics_{repeat_idx}.json")
with open(metrics_path, "w") as f:
json.dump(metrics, f, indent=4)

sorted_filewise = sorted(filewise_metrics, key=lambda x: x.get('cer', 0), reverse=True)
with open(os.path.join(eval_dir, f"{dataset}_filewise_metrics_{repeat_idx}.json"), "w") as f:
json.dump(sorted_filewise, f, indent=4)
filewise_metrics_path = os.path.join(eval_dir, f"{dataset}_filewise_metrics_{repeat_idx}.json")
with open(filewise_metrics_path, "w", encoding="utf-8") as f:
json.dump(sorted_filewise, f, indent=4, ensure_ascii=False)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Open the JSON file with an explicit UTF-8 encoding.

ensure_ascii=False writes Unicode characters directly, but the surrounding open(..., "w") uses the platform locale. On non-UTF-8 environments this can raise UnicodeEncodeError or corrupt the intended raw text.

Proposed fix
-            with open(os.path.join(eval_dir, f"{dataset}_filewise_metrics_{repeat_idx}.json"), "w") as f:
+            with open(
+                os.path.join(eval_dir, f"{dataset}_filewise_metrics_{repeat_idx}.json"),
+                "w",
+                encoding="utf-8",
+            ) as f:
                 json.dump(sorted_filewise, f, indent=4, ensure_ascii=False)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
json.dump(sorted_filewise, f, indent=4, ensure_ascii=False)
with open(
os.path.join(eval_dir, f"{dataset}_filewise_metrics_{repeat_idx}.json"),
"w",
encoding="utf-8",
) as f:
json.dump(sorted_filewise, f, indent=4, ensure_ascii=False)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/tts/magpietts_inference.py` at line 346, Update the JSON output file
opening in the code surrounding json.dump to explicitly use UTF-8 encoding,
while preserving ensure_ascii=False and the existing serialization behavior.


# Append to per-run CSV
append_metrics_to_csv(per_run_csv, full_checkpoint_name, dataset, metrics)
Expand Down
12 changes: 6 additions & 6 deletions scripts/tts_comparison_report/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ python scripts/tts_comparison_report/generate_report.py \
--s3_region us-west-2 \
--task_id NEMOTTS-2007 \
--audio_report \
--audio_report_benchmarks libritts_test_clean,riva_hard_digits \
--audio_report_benchmarks libritts,riva_en \
--samples_per_benchmark 20
```

Expand All @@ -145,7 +145,7 @@ python scripts/tts_comparison_report/generate_report.py \
--s3_region us-west-2 \
--task_id NEMOTTS-2007 \
--audio_report \
--audio_report_benchmarks libritts_test_clean,riva_hard_digits \
--audio_report_benchmarks libritts,riva_en \
--samples_per_benchmark 20 \
--remote_hostname your_remote_host \
--remote_username your_user
Expand All @@ -160,13 +160,13 @@ python scripts/tts_comparison_report/generate_report.py \
--baseline_path /workspace/NeMo/exp/buckets/baseline \
--candidate_name "Model B" \
--candidate_path /workspace/NeMo/exp/buckets/candidate \
--benchmarks libritts_test_clean,riva_hard_digits,riva_hard_letters \
--benchmarks libritts,riva_en,riva_en_hard_sentences \
--s3_endpoint https://your-s3-endpoint \
--s3_bucket your_bucket_name \
--s3_region us-west-2 \
--task_id NEMOTTS-2007 \
--audio_report \
--audio_report_benchmarks libritts_test_clean,riva_hard_digits \
--audio_report_benchmarks libritts,riva_en \
--samples_per_benchmark 20
```

Expand All @@ -187,8 +187,8 @@ The default expiration time is one year.
- The expiration time is also included as a suffix in the uploaded artifacts
directory name, using the format `%Y-%m-%dT%H-%M-%SZ`, so uploaded reports
can be filtered and deleted later if needed.
- Both generated reports include a clickable Jira link derived from `--task_id`.
If no task ID is specified, the link points to the Jira project page.
- Both generated reports include a clickable POR link derived from `--task_id`.
If no task ID is specified, the link points to the POR project page.
Comment thread
artem-gorodetskii marked this conversation as resolved.

## Maintenance

Expand Down
6 changes: 3 additions & 3 deletions scripts/tts_comparison_report/generate_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
_S3_ACCESS_KEY_ID: str = "S3_ACCESS_KEY_ID"
_S3_SECRET_ACCESS_KEY: str = "S3_SECRET_ACCESS_KEY"

_DEFAULT_BENCHMARK_NAMES: str = ",".join(SUPPORTED_BENCHMARK_NAMES)
_DEFAULT_BENCHMARK_NAMES: str = "libritts,riva_en"


def _create_argparser() -> ArgumentParser:
Expand Down Expand Up @@ -113,7 +113,7 @@ def _create_argparser() -> ArgumentParser:
"--task_id",
type=str,
default=DUMMY_TASK_ID,
help="Jira task number associated with this report.",
help="Task ID associated with this report.",
)
parser.add_argument(
"--results_subdir",
Expand All @@ -129,7 +129,7 @@ def _create_argparser() -> ArgumentParser:
parser.add_argument(
"--audio_report_benchmarks",
type=str,
default="libritts_test_clean,riva_hard_digits,riva_hard_letters",
default=_DEFAULT_BENCHMARK_NAMES,
help="Comma-separated list of benchmarks to include in the audio report.",
)
parser.add_argument(
Expand Down
41 changes: 28 additions & 13 deletions scripts/tts_comparison_report/reporting/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,30 @@
_ROOT: Path = Path(__file__).parent.parent

# Benchmark names supported by the comparison report pipeline.
SUPPORTED_BENCHMARK_NAMES: list[str] = [
"libritts_seen",
"libritts_test_clean",
"riva_hard_digits",
"riva_hard_letters",
"riva_hard_money",
"riva_hard_short",
"vctk",
]
BENCHMARK_META: dict[str, str] = {
'libritts': 'en',
'riva_en': 'en',
'riva_en_hard_sentences': 'en',
'riva_en_short_sentences': 'en',
'riva_en_qa': 'en',
'riva_en_qa_longform': 'en',
'King_ASR_sa_diacritics': 'ar',
'King_ASR_sa_no_diacritics': 'ar',
'King_ASR_uae_diacritics': 'ar',
'King_ASR_uae_no_diacritics': 'ar',
'cmltts_de': 'de',
'cmltts_es': 'es',
'cmltts_fr': 'fr',
'AI4bharat': 'hi',
'cmltts_it': 'it',
'jvs_jsut': 'ja',
'F5I9N7A1': 'ko',
'cmltts_pt': 'pt',
'vivos': 'vi',
'mscenespeech': 'zh',
}

SUPPORTED_BENCHMARK_NAMES: list[str] = list(BENCHMARK_META.keys())

# Default width of tqdm progress bars in terminal columns.
TQDM_NCOLS: int = 80
Expand All @@ -48,8 +63,8 @@
# Directory containing Jinja templates used for report rendering.
TEMPLATES_DIR: Path = _ROOT / "templates"

# Fallback task id used when no real Jira ticket is provided.
DUMMY_TASK_ID: str = "NEMOTTS-0000"
# Fallback task id used when no real ticket is provided.
DUMMY_TASK_ID: str = "NMP-I-000"

# URL prefix used to construct clickable Jira ticket links in reports.
JIRA_TICKET_URL_PREFIX: str = "https://jirasw.nvidia.com/browse"
# URL prefix used to construct clickable ticket links in reports.
TICKET_URL_PREFIX: str = "https://modelpor.ideas.aha.io/ideas"
14 changes: 6 additions & 8 deletions scripts/tts_comparison_report/reporting/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from datetime import UTC, datetime, timedelta
from pathlib import Path

from scripts.tts_comparison_report.reporting.constants import DUMMY_TASK_ID, JIRA_TICKET_URL_PREFIX
from scripts.tts_comparison_report.reporting.constants import TICKET_URL_PREFIX
from scripts.tts_comparison_report.reporting.models import ExpirationInfo, TaskInfo


Expand All @@ -37,21 +37,19 @@ def make_expiration_info(expires_in: int) -> ExpirationInfo:


def make_task_info(task_id: str) -> TaskInfo:
"""Create task metadata and the corresponding Jira link information.
"""Create task metadata and the corresponding link information.

Args:
task_id: Jira task identifier used for the report.
task_id: Task identifier used for the report.

Returns:
Task information with the original task ID, derived Jira ID, and Jira URL.
Task information with the original task ID and task URL.
"""
jira_id = task_id if task_id != DUMMY_TASK_ID else task_id.split("-")[0]
jira_url = f"{JIRA_TICKET_URL_PREFIX}/{jira_id}"
task_url = f"{TICKET_URL_PREFIX}/{task_id}"

return TaskInfo(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The previous workflow was:

  • task_id was the internal task identifier. If not specified, it defaulted to DUMMY_TASK_ID. It was used in the S3 report name and this naming is important for maintenance: it allows us to remove reports for old tasks and to deprioritize reports without explicit task IDs.
  • jira_id was derived from task_id and used only for display in the report sidebar. If DUMMY_TASK_ID was used, jira_id became NEMOTTS, so the report still showed our Jira space.
  • jira_url was derived from JIRA_TICKET_URL_PREFIX and jira_id, so it always pointed to a valid page: either the real Jira ticket or, when no task ID was provided, the Jira project page.

Currently, task_id is being used for all three purposes:

  • S3 report naming,
  • displayed task ID in the report,
  • and construction of task_url.

As a result, when task_id is not specified, the report shows DUMMY_TASK_ID and generates a broken POR link.

If we want to keep task_id optional, the previous behavior should be preserved. In other words, we should keep separate values for:

  • the identifier used in S3 naming (actual POR ID or NMP-I-000),
  • the identifier displayed in the report (actual POR ID or something like NMP-I or NMP),
  • and the clickable URL (real POR ticket URL or the default TICKET_URL_PREFIX).

Sorry for the earlier misunderstanding.

task_id=task_id,
jira_id=jira_id,
jira_url=jira_url,
task_url=task_url,
)


Expand Down
5 changes: 2 additions & 3 deletions scripts/tts_comparison_report/reporting/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -535,11 +535,10 @@ def get_benchmark_sample_meta(

@dataclass(frozen=True)
class TaskInfo:
"""Task identifiers and derived Jira link information used in reports."""
"""Task identifiers and derived link information used in reports."""

task_id: str
jira_id: str
jira_url: str
task_url: str


@dataclass(frozen=True)
Expand Down
19 changes: 12 additions & 7 deletions scripts/tts_comparison_report/reporting/orchestrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
prepare_eval_artifacts,
)
from scripts.tts_comparison_report.reporting.constants import (
BENCHMARK_META,
S3_AUDIO_DIR,
S3_IMAGES_DIR,
S3_LINK_EXPIRES_IN,
Expand Down Expand Up @@ -268,12 +269,14 @@ def _render_audio_report(
pair_blocks=pair_blocks,
)
benchmark_blocks.append(block)
benchmark_section_info.append((benchmark_name, benchmark_name))
benchmark_language = BENCHMARK_META[benchmark_name]
name_info = f"{benchmark_name} ({benchmark_language})"
benchmark_section_info.append((benchmark_name, name_info))

report = self.renderer.render(
name=TemplateName.audio_report,
jira_id=task_info.jira_id,
jira_url=task_info.jira_url,
task_id=task_info.task_id,
task_url=task_info.task_url,
header_block=header_block,
benchmark_blocks=benchmark_blocks,
benchmark_section_info=benchmark_section_info,
Expand Down Expand Up @@ -370,13 +373,15 @@ def _render_eval_report(
image_block=image_block,
)
benchmark_blocks.append(block)
benchmark_section_info.append((benchmark_name, benchmark_name))
benchmark_language = BENCHMARK_META[benchmark_name]
name_info = f"{benchmark_name} ({benchmark_language})"
benchmark_section_info.append((benchmark_name, name_info))

report = self.renderer.render(
name=TemplateName.eval_report,
is_self_comparison=eval_artifacts.is_self_comparison,
jira_id=task_info.jira_id,
jira_url=task_info.jira_url,
task_id=task_info.task_id,
task_url=task_info.task_url,
audio_report_url=audio_report_url,
configuration_block=configuration_block,
header_block=header_block,
Expand Down Expand Up @@ -414,7 +419,7 @@ def run(
generate_audio_report: Whether to generate the audio comparison report.
audio_report_benchmarks: Benchmark names to include in the audio report.
samples_per_benchmark: Number of audio pairs to sample per benchmark.
task_id: Task identifier used for report metadata and Jira linking.
task_id: Task identifier used for report metadata and link.

Returns:
Tuple containing the evaluation report URL and the optional audio report URL.
Expand Down
2 changes: 1 addition & 1 deletion scripts/tts_comparison_report/templates/audio_report.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@
<h2>Links ↗</h2>
<ul>
<li class="link-comment">
<a href="{{ jira_url }}" target="_blank" rel="noopener noreferrer">Jira {{ jira_id }}</a>
<a href="{{ task_url }}" target="_blank" rel="noopener noreferrer">POR {{ task_id }}</a>
</li>
</ul>
</aside>
Expand Down
2 changes: 1 addition & 1 deletion scripts/tts_comparison_report/templates/eval_report.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@
<h2>Links ↗</h2>
<ul>
<li class="link-comment">
<a href="{{ jira_url }}" target="_blank" rel="noopener noreferrer">Jira {{ jira_id }}</a>
<a href="{{ task_url }}" target="_blank" rel="noopener noreferrer">POR {{ task_id }}</a>
</li>
{% if audio_report_url %}
<li class="link-comment">
Expand Down
Loading