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

importing lrc with multiple cpu measurements fails #1

Open
olichtne opened this issue Nov 14, 2022 · 3 comments
Open

importing lrc with multiple cpu measurements fails #1

olichtne opened this issue Nov 14, 2022 · 3 comments

Comments

@olichtne
Copy link
Contributor

olichtne commented Nov 14, 2022

https://github.com/LNST-project/lrc-file/blob/main/lrc_file/LrcFile.py#L86 assumes that the lrc file contains EXACTLY two cpu measurements. If there's only one, or if there's more than one (e.g. for OvSDPDKPvPRecipe there's 3) the LrcFile load crashes with an exception.

This will likely need a slightly larger refactoring and thought to put into how this is implemented as this cascades into other parts of the implementation.

@olichtne
Copy link
Contributor Author

fyi @Kuba314

@Kuba314
Copy link
Contributor

Kuba314 commented Nov 14, 2022

This choice of only 2 machines is influenced by the fact that we distinguish between a generator and a receiver machine. There has to be a way to distinguish the machines between each other in the .lrc files.

@olichtne
Copy link
Contributor Author

we distinguish between a generator and a receiver machine

no, that's a misunderstanding from implementation specifics for some of our use cases. For CPU measurements, there is no concept of a generator or a receiver, there's only the Host that is being measured. The measurement does have its configuration which contains the information about which host is being measured and that's it.

For some of our internal use cases we've re-named some of the metrics in some places to map it a bit closer to the reality of what we're measuring, but this doesn't represent the real generalization or design behind this.


On a related note, this issue is not only about the fact that sometimes we may have 3 machines which we measure instead of 2. A recipe may have multiple smaller "tests" in it... for example in our ENRT package, the tests support looping over multiple sub configurations in a single test run. At the moment we don't use this feature internally, however this is an intended feature that can save significant amounts of test time that is spent on configuring-deconfiguring loops for similar tests.

In a case where we would use this feature even for recipes where we measure only the generator-receiver host pair, we'd still end up in a situation where filter(_is_cpu_measurement_result, lnst_run.results) would return N*2 cpu measurement results based on how many sub configuration loops we did.


This is the main reason why i mention that this will most likely require a larger refactoring as this assumption that we've made a long time ago for how to implement the LrcFile class cascades into many other places and it's an incorrect assumption.

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

No branches or pull requests

2 participants