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

gt4py tests: refactor StencilTestSuite base class to avoid test dependencies #72

Open
romanc opened this issue Feb 7, 2025 · 0 comments

Comments

@romanc
Copy link

romanc commented Feb 7, 2025

Description

In the gt4py test suite, StencilTestSuite provides convenient base class for defining integration tests for stencils. Given the stencil function and input / validation arguments, the class will setup

  1. a first test which lowers the stencil into backend code and does sanity checks on the generated code
  2. a second test which calls the generated stencil with the provided input arguments and validates the results of the computation with the validation arguments

To avoid re-compiling the stencil in the second test, it re-uses the cached stencil of the first test. This means generation tests need to run before validation tests can run. This test order dependency complicates test parallelization and PR GridTools/gt4py#1849 works around the issue with minimal changes to the test infrastructure.

In my opinion, it would be simpler to scratch the idea of separating generation and validation tests. The idea would be to have only one test per stencil function. That test would first lower the stencil and then validate the implementation all in one test call. If code generation fails, validation is automatically skipped and the result of code generation is naturally re-used without the need to store it in the test context. Parallelization would be trivial because of the removed order dependency.

The drawbacks of the proposed solutions include:

  • StencilTestSuite isn't straight forward to understand. The class looks over-designed for the solution proposed in this issue and it might be easier to write a new base class from scratch with the revised requirements in mind.
  • The current solution is working fine and the workaround from PR ci[cartesian]: Thread safe parallel stencil tests GridTools/gt4py#1849 addresses the issue relatively cleanly at the base class level. It seems this will land very low on the priority scale.

This is a follow-up from PR GridTools/gt4py#1849.

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

1 participant