Skip to content

Conversation

@mszpindler
Copy link
Contributor

I would like to propose optional validation parameter for the checks class that enables built-in OSU validation.

@jenkins-cscs
Copy link
Collaborator

Can I test this patch?

@ekouts ekouts requested review from rsarm and vkarak October 21, 2022 11:52
@rsarm rsarm changed the title Add optional validation -c option to the OSU tests [test] Add optional validation -c option to the OSU hpclib tests Oct 21, 2022
@ekouts
Copy link
Contributor

ekouts commented Oct 21, 2022

Ok to test

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

That's a good proposal and I would propose to replace our current validation completely. It's always better to use the benchmark's validation. @rsarm What do you think?

@rsarm
Copy link
Contributor

rsarm commented Oct 24, 2022

I agree. I think it's a good idea. @mszpindler had told me about it offline.

#:
#: :type: `bool`
#: :default: ``False``
validation = variable(bool, value=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we should not use an extra variable then and always run with the -c option. Therefore, we should also keep only the new validation test.

@vkarak
Copy link
Contributor

vkarak commented Nov 16, 2022

@mszpindler I have made a PR (Lumi-supercomputer#8) to your branch with my suggestion to use always the -c option. Could you please merge it, so that we can merge this one as well?

@vkarak
Copy link
Contributor

vkarak commented Nov 19, 2022

Replaced by #2679.

@vkarak vkarak closed this Nov 19, 2022
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.

5 participants