inducer / arraycontext

Choose your favorite numpy-workalike!
6 stars 11 forks source link

Restore loop inference fallback #269

Closed matthiasdiener closed 3 months ago

matthiasdiener commented 3 months ago

This reverts a part of #264 (in particular, parts of https://github.com/inducer/arraycontext/pull/264/commits/9af49ba027b6bf6c9c48a31d28a5a8860869f9bc).

With this PR (and something like https://github.com/illinois-ceesd/mirgecom/pull/898 https://github.com/illinois-ceesd/mirgecom/pull/1048), mirgecom is able to run from the main branches of the software stack.

Side note: We carry a similar patch as this PR also in our production branch: https://github.com/illinois-ceesd/arraycontext/commit/bfd22a50808622cdcca8afb415bba20dbaabf058

Related: https://github.com/inducer/meshmode/issues/333 (?)

Please squash

inducer commented 3 months ago

What tests are failing without this? And: couldn't you just use the meshmode array context in those examples?

matthiasdiener commented 3 months ago

What tests are failing without this? And: couldn't you just use the meshmode array context in those examples?

The pattern of failures looks like: (full log)

In test/test_bc.py: _ test_normal_axes_utility[<PyOpenCLArrayContext for <pyopencl.Device 'cpu' on 'Portable Computing Language'>>-1] _

test/test_bc.py:95: in vec_norm
    return actx.to_numpy(op.norm(dcoll, vec, p=p)) # noqa

[...]

../grudge/grudge/geometry/metrics.py:171: in forward_metric_nth_derivative
    vec = num_reference_derivative(
../meshmode/meshmode/discretization/__init__.py:721: in num_reference_derivative
    return _DOFArray(actx, tuple(
../meshmode/meshmode/discretization/__init__.py:722: in <genexpr>
    actx.einsum("ij,ej->ei",
../arraycontext/arraycontext/context.py:497: in einsum
    out_ary = self.call_loopy(
../arraycontext/arraycontext/impl/pyopencl/__init__.py:275: in call_loopy
    executor = self.transform_loopy_program(t_unit).executor(self.context)

[...]

        else:
>           raise RuntimeError(
                "Unable to reason what outer_iname and inner_iname "
                f"needs to be; all_inames is given as: {all_inames}"
            )
E           RuntimeError: Unable to reason what outer_iname and inner_iname needs to be; all_inames is given as: frozenset({'e', 'i', 'j'})
inducer commented 3 months ago

Have you tried something along these lines?

diff --git a/test/test_bc.py b/test/test_bc.py
index f34f1776..5fd76890 100644
--- a/test/test_bc.py
+++ b/test/test_bc.py
@@ -52,9 +52,10 @@ from mirgecom.gas_model import (
 import grudge.op as op
 from mirgecom.simutil import get_box_mesh

-from meshmode.array_context import (  # noqa
-    pytest_generate_tests_for_pyopencl_array_context
-    as pytest_generate_tests)
+from meshmode.array_context import PytestPyOpenCLArrayContextFactory
+from arraycontext import pytest_generate_tests_for_array_contexts
+pytest_generate_tests = pytest_generate_tests_for_array_contexts(
+        [PytestPyOpenCLArrayContextFactory])

 logger = logging.getLogger(__name__)

As recommended by the DeprecationWarnings:

  /home/andreas/src/mirgecom/test/test_wave.py:34: DeprecationWarning: meshmode.array_context.pytest_generate_tests_for_pyopencl_array_context is deprecated. Use arraycontext.pytest_generate_tests_for_pyopencl_array_context instead. meshmode.array_context.pytest_generate_tests_for_pyopencl_array_context will continue to work until 2022.
matthiasdiener commented 3 months ago

Have you tried something along these lines? [...]

Yes, this causes the same error.

inducer commented 3 months ago

:shrug: I just tried the patch above in a fresh emirge install, and it fixes the issue for me. Note that this needs to be done in every test file.

matthiasdiener commented 3 months ago

🤷 I just tried the patch above in a fresh emirge install, and it fixes the issue for me. Note that this needs to be done in every test file.

Weird, this patch does seem to work in CI without this PR. Maybe something wrong in my environment.

matthiasdiener commented 3 months ago

Closing, as it seems to be unnecessary.