-
Notifications
You must be signed in to change notification settings - Fork 935
[Profile] Adding metrics for Diffusion/DiT Single diffusion Pipeline #668
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
Changes from all commits
39fe587
76b1739
1464bc1
08e6f59
265999d
a3bcdcb
d7175bf
a9a518b
02e0a82
5d909d5
9c59a51
6148be5
3fba139
17a6342
5ab4d3c
f6c99ef
88837ca
dd5094a
a0bd30d
9b89175
9af8a06
f78a99b
cf7dbc2
6d0272d
4ac92fb
64a4f68
36be0da
479768c
57b9fac
4b3dbf3
529c2d7
d0dd690
0b88e65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1135,6 +1135,15 @@ def _run_generation( | |
| final_output_type=stage.final_output_type, # type: ignore[attr-defined] | ||
| request_output=engine_outputs, | ||
| ) | ||
| try: | ||
| if stage.final_output_type == "text" or metrics.log_stats: | ||
| output_to_yield.metrics = metrics.build_output_metrics(stage_id, req_id) | ||
| except Exception as e: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bare
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Much better — logging the exception and setting explicit empty dict. |
||
| # Make metrics contract explicit on failure. | ||
| output_to_yield.metrics = {} | ||
| logger.exception( | ||
| f"[{self._name}] Failed to attach output metrics for req {req_id} at stage {stage_id}: {e}", | ||
| ) | ||
|
|
||
| # Record audio generated frames (only when finished) | ||
| try: | ||
|
|
||
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.
This indentation change moves
return resultsout of theelsebranch -- was this intentional? Double-check the single-prompt case still returns correctly.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.
Checked — the single-prompt path returns early, so this is fine.