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

[NPUW] Serialization tests #29077

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

smirnov-alexey
Copy link
Contributor

No description provided.

@smirnov-alexey smirnov-alexey requested review from a team as code owners February 19, 2025 19:04
@github-actions github-actions bot added category: NPU OpenVINO NPU plugin category: NPUW NPUW plugin labels Feb 19, 2025
@dmatveev dmatveev added this to the 2025.1 milestone Feb 19, 2025
Copy link
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

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

Overall this is some very basic tests, do you plan to add more? E.g., serialize/deserialize a tiny synthetic model? Not necessarily an LLM.

Comment on lines 144 to 160
void ov::npuw::s11n::read(std::istream& stream, ov::npuw::compiled::Spatial& var) {
using ov::npuw::s11n::read;

ov::npuw::compiled::Spatial spat;
std::size_t params_size = 0;
read(stream, params_size);
for (std::size_t i = 0; i < params_size; ++i) {
ov::npuw::compiled::Spatial::Param p;
read(stream, p.idx);
read(stream, p.dim);
spat.params.push_back(p);
var.params.push_back(p);
}
read(stream, spat.range);
read(stream, spat.nway);
read(stream, spat.out_dim);
read(stream, spat.nway_iters);
read(stream, spat.tail_size);
read(stream, var.range);
read(stream, var.nway);
read(stream, var.out_dim);
read(stream, var.nway_iters);
read(stream, var.tail_size);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like our AOT spatial blobs didn't work till this very moment :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It did. Only SPATIAL config was broken

Comment on lines 147 to 156
int tmp = 42;
ov::Any var(tmp);
ov::Any res;

std::stringstream ss;

write_any(ss, var);
read_any(ss, res);

EXPECT_EQ(var.as<int>(), res.as<int>());
Copy link
Contributor

Choose a reason for hiding this comment

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

should it have more types? at least two more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

TEST(SerializationTest, BasicTypes_array) {
using namespace ov::npuw::s11n;

std::array<uint8_t, 6> res;
Copy link
Contributor

Choose a reason for hiding this comment

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

it must have its own type. at least a typedef

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines +260 to +261
write_weightless(ss, {var}, ctx);
read_weightless(ss, res, nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

test says "with_weights"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but there is no way to test proper weightless mode - we don't have a model - thus no weights. write_weightsless does support both cases - it tries to write as weightless, but if it's not possible - it will serialize full Tensor

@smirnov-alexey
Copy link
Contributor Author

Overall this is some very basic tests, do you plan to add more? E.g., serialize/deserialize a tiny synthetic model? Not necessarily an LLM.

I agree, but I had to remove several tests due to API and lack of model limitations. I can't test weights bank or LazyTensor as a separate entity. I will try to add a test on dummy model export/import

@smirnov-alexey
Copy link
Contributor Author

Overall this is some very basic tests, do you plan to add more? E.g., serialize/deserialize a tiny synthetic model? Not necessarily an LLM.

I agree, but I had to remove several tests due to API and lack of model limitations. I can't test weights bank or LazyTensor as a separate entity. I will try to add a test on dummy model export/import

Not possible to add dummy model test on LLMCompiledModel due to internal passes and CompiledModel doesn't support export. I suggest it to be added separately in the next PR

@github-actions github-actions bot added the category: build OpenVINO cmake script / infra label Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: build OpenVINO cmake script / infra category: NPU OpenVINO NPU plugin category: NPUW NPUW plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants