Skip to content
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

Add LongBench validation #1220

Merged
merged 18 commits into from
Feb 7, 2025
Merged

Conversation

l-bat
Copy link
Contributor

@l-bat l-bat commented Nov 18, 2024

No description provided.

@l-bat l-bat requested a review from vshampor November 18, 2024 08:52
@l-bat l-bat marked this pull request as draft November 18, 2024 08:52
@github-actions github-actions bot added category: continuous batching Continuous batching category: sampling Sampling / Decoding algorithms labels Nov 18, 2024
@ilya-lavrenov ilya-lavrenov removed the category: sampling Sampling / Decoding algorithms label Nov 20, 2024
@l-bat l-bat force-pushed the lt/eval_longbench branch from 6c96bd0 to eefe4f2 Compare December 18, 2024 18:45
@l-bat l-bat marked this pull request as ready for review December 19, 2024 10:25
@ilya-lavrenov
Copy link
Contributor

is it still valid?
Please, either finalize or close the PR.

@l-bat l-bat force-pushed the lt/eval_longbench branch from 000d2a4 to 7eb1e54 Compare January 7, 2025 09:39
@l-bat l-bat force-pushed the lt/eval_longbench branch from 6c3e950 to becdf78 Compare January 31, 2025 15:00
@github-actions github-actions bot added the category: GHA CI based on Github actions label Feb 3, 2025
@l-bat l-bat force-pushed the lt/eval_longbench branch from d71206d to 15d3c77 Compare February 3, 2025 14:35
@github-actions github-actions bot removed the category: GHA CI based on Github actions label Feb 3, 2025
@ilya-lavrenov ilya-lavrenov added this to the 2025.1 milestone Feb 4, 2025
LongBenchTestData("trec", 3.2, 2.0, 3.3),
LongBenchTestData("qasper", 5.8, 1.7, 3.6),
])
def test_optimized_generation_longbench(qwen2_converted_model, test_struct):
Copy link
Contributor

Choose a reason for hiding this comment

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

is it explicitly not marked as @pytest.mark.precommit ?
how long does this test take?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes
On my local machine it takes 24 minutes, but on CI it takes about 60 minutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we run this test in nightly jobs?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, please add mark for nightly

@@ -0,0 +1,245 @@
import re
Copy link
Collaborator

Choose a reason for hiding this comment

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

This code is probably taken from 3rd party repo. We need to keep the license in this case.

Choose a reason for hiding this comment

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

I would suggest to check if Protex will actually catch it. In general it's not a problem if the license allows reusing code. In that case you need clearly state that in your file.

@l-bat l-bat force-pushed the lt/eval_longbench branch from 15d3c77 to 1e88d10 Compare February 4, 2025 09:08
Comment on lines +1 to +8
# Copyright (C) 2023-2025 Intel Corporation
# SPDX-License-Identifier: Apache-2.0
#
# This file includes utility functions copied from the LongBench repository:
# https://github.com/THUDM/LongBench
#
# Copyright (c) 2023 THU-KEG & Zhipu AI
# Licensed under the MIT License
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@moslex please take a look at the license. Could you confirm if it is sufficient for code adapted from a 3rd party repo?

Copy link

Choose a reason for hiding this comment

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

Yes, you need to keep the original copyrights and should mark the entire file as Apache 2.0 to be compliant with the OpenVINO license. This is in case you copy-paste some code and make your own changes. Everything is good now.

If you copy-paste the full file/component without any modifications, it is better to keep its original license and move this file/component to a separate folder named "thirdparty" and update the thirdparty.txt file by adding the license for this component.

Choose a reason for hiding this comment

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

@l-bat , you can also check how it's done in NNCF. I can guide you if needed.

@AlexKoff88 AlexKoff88 added this pull request to the merge queue Feb 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 6, 2025
@l-bat l-bat force-pushed the lt/eval_longbench branch from 315576d to 85b5e32 Compare February 6, 2025 19:08
@AlexKoff88 AlexKoff88 added this pull request to the merge queue Feb 7, 2025
Merged via the queue into openvinotoolkit:master with commit e6b0b20 Feb 7, 2025
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants