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

Restore loop inference fallback #269

Closed

Conversation

matthiasdiener
Copy link
Collaborator

@matthiasdiener matthiasdiener commented Jul 22, 2024

This reverts a part of #264 (in particular, parts of 9af49ba).

With this PR (and something like illinois-ceesd/mirgecom#898 illinois-ceesd/mirgecom#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: illinois-ceesd@bfd22a5

Related: inducer/meshmode#333 (?)

Please squash

@matthiasdiener matthiasdiener self-assigned this Jul 22, 2024
matthiasdiener added a commit to illinois-ceesd/mirgecom that referenced this pull request Jul 22, 2024
matthiasdiener added a commit to illinois-ceesd/mirgecom that referenced this pull request Jul 23, 2024
@inducer
Copy link
Owner

inducer commented Jul 23, 2024

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

@matthiasdiener
Copy link
Collaborator Author

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
Copy link
Owner

inducer commented Jul 23, 2024

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
Copy link
Collaborator Author

matthiasdiener commented Jul 23, 2024

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

Yes, this causes the same error.

@inducer
Copy link
Owner

inducer commented Jul 23, 2024

🤷 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
Copy link
Collaborator Author

🤷 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 matthiasdiener marked this pull request as draft July 24, 2024 21:44
@matthiasdiener
Copy link
Collaborator Author

Closing, as it seems to be unnecessary.

@matthiasdiener matthiasdiener deleted the restore-loop-inf-fallback branch July 26, 2024 14:43
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

Successfully merging this pull request may close these issues.

2 participants