Skip to content

Make timeouts affect metrics performance#304

Open
nck-mlcnv wants to merge 19 commits into
developfrom
feature/metric-calculations
Open

Make timeouts affect metrics performance#304
nck-mlcnv wants to merge 19 commits into
developfrom
feature/metric-calculations

Conversation

@nck-mlcnv
Copy link
Copy Markdown
Contributor

The AvgQPS and QMPH metric should take in timeouts in their result.

@nck-mlcnv nck-mlcnv requested a review from nkaralis August 22, 2025 07:55
Copy link
Copy Markdown

@nkaralis nkaralis left a comment

Choose a reason for hiding this comment

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

Thank you. I have some comments

Comment thread docs/configuration/metrics.md
Comment thread src/main/java/org/aksw/iguana/cc/metrics/impl/QMPH.java
Comment thread src/main/java/org/aksw/iguana/cc/metrics/impl/QPS.java
Copy link
Copy Markdown

@nkaralis nkaralis left a comment

Choose a reason for hiding this comment

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

Penalty needs to be applied for all timed out and failed queries (e.g., error code).

Comment thread docs/configuration/metrics.md
Copy link
Copy Markdown

@nkaralis nkaralis left a comment

Choose a reason for hiding this comment

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

Also, please define timeouts and penalties in the readme.

Comment thread src/main/java/org/aksw/iguana/cc/metrics/impl/PQMPH.java Outdated
Comment thread src/main/java/org/aksw/iguana/cc/metrics/impl/PQMPH.java
Comment thread src/main/java/org/aksw/iguana/cc/metrics/impl/PQMPH.java Outdated
Comment thread docs/configuration/metrics.md
@nck-mlcnv nck-mlcnv requested a review from nkaralis October 17, 2025 09:21
Copy link
Copy Markdown

@nkaralis nkaralis left a comment

Choose a reason for hiding this comment

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

Please write test cases for the new metrics

Comment thread docs/configuration/metrics.md Outdated
Comment thread docs/configuration/metrics.md
Comment thread src/main/java/org/aksw/iguana/cc/metrics/impl/PQMPH.java Outdated
Comment thread src/main/java/org/aksw/iguana/cc/metrics/impl/PNoQPH.java Outdated
@nck-mlcnv nck-mlcnv force-pushed the feature/metric-calculations branch from 2f5c0e7 to af9e091 Compare November 14, 2025 14:25
@nck-mlcnv nck-mlcnv requested a review from nkaralis November 14, 2025 15:18
Copy link
Copy Markdown

@nkaralis nkaralis left a comment

Choose a reason for hiding this comment

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

Thank you for adding the test cases.
I have one last request. Include a timeout and error in the test cases to ensure that the penalized metrics work properly. Add comments in the test cases to make them easier to follow.

@nck-mlcnv nck-mlcnv requested a review from nkaralis December 2, 2025 17:39
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.

2 participants