Skip to content

New metric: ksim - #75

Merged
rcannood merged 9 commits into
mainfrom
features/ksim
Aug 21, 2026
Merged

New metric: ksim#75
rcannood merged 9 commits into
mainfrom
features/ksim

Conversation

@mumichae

@mumichae mumichae commented Aug 18, 2025

Copy link
Copy Markdown
Collaborator

Describe your changes

Checklist before requesting a review

  • I have performed a self-review of my code

  • Check the correct box. Does this PR contain:

    • Breaking changes
    • New functionality
    • Major changes
    • Minor changes
    • Bug fixes
  • Proposed changes are described in the CHANGELOG.md

  • CI Tests succeed and look good!

Comment thread scripts/create_component/create_python_metric.sh Outdated
@mumichae
mumichae marked this pull request as ready for review August 29, 2025 10:05
@mumichae mumichae changed the title Features/ksim New metric: ksim Aug 29, 2025
@seohyonkim

Copy link
Copy Markdown
Contributor

@mumichae it's good to go! Please check out whenever you have time :)

@mumichae mumichae left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Looking good! If you update the metric description, it should be ready to merge

Comment thread src/metrics/ksim/config.vsh.yaml Outdated
Comment thread CHANGELOG.md Outdated

* Added `metrics/kbet_pg` and `metrics/kbet_pg_label` components (PR #52).

* Added `metircs/ksim` component (PR #75).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
* Added `metircs/ksim` component (PR #75).
* Added `metrics/ksim` component (PR #75).

Comment thread src/metrics/ksim/config.vsh.yaml Outdated
* Output both `ksim_mean` and `ksim_accept_rate` -- `calc_kSIM()` returns a tuple, so writing it as one value produced a 1x2 `metric_values` array
* Add missing `metric_type: embedding`, without which `run_benchmark` filters the metric out
* Register `metrics/ksim` in the `run_benchmark` workflow
* Drop the tunable arguments in favour of pegasus' defaults (K=25, min_rate=0.9)
* Use `meta["cpus"]` instead of every physical core on the node
* Align the docker image and pypi dependencies with `metrics/kbet_pg`
* Add the `## VIASH START` block, rephrase the descriptions in passive voice, fix the changelog typo
@rcannood

Copy link
Copy Markdown
Member

LGTM!

@rcannood
rcannood merged commit 982fed5 into main Aug 21, 2026
2 checks passed
@rcannood
rcannood deleted the features/ksim branch August 21, 2026 15:46
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.

3 participants