multi-domain assembly - #4763
Conversation
connorjward
left a comment
There was a problem hiding this comment.
I wonder if all this logic should simply be
|
Don't merge, it isn't right yet |
Please leave this as a draft PR then |
dd2adba to
e74ad9b
Compare
Co-authored-by: David A. Ham <david.ham@imperial.ac.uk>
|
@connorjward Can this be merged? The errors look unrelated. |
|
I agree. @UZerbinati it looks like your merges into ngsPETSc have broken things. I will try to fix them when I move things over (imminently). |
I don't see what I've broken, all Firedrake test's passed in ngsPETSc before merging. @connorjward so the incriminated line seems to be: Also this error is not reproducible in my local machine with a new Firedrake installation :( |
They passed on Firedrake
See #4629 |
|
I fixed this for ngsPETSc in NGSolve/ngsPETSc#94, before you made more changes. It's fine. Release still works which is the most important thing. I'll sort this all out tomorrow hopefully. |
| def test_multi_domain_assemble(): | ||
| mesh1 = UnitSquareMesh(1, 1, quadrilateral=True) | ||
| mesh2 = UnitSquareMesh(2, 2) | ||
| V1 = FunctionSpace(mesh1, "Q", 1) | ||
| V2 = FunctionSpace(mesh2, "CG", 1) | ||
| V = V1 * V2 | ||
|
|
||
| u = TrialFunctions(V) | ||
| v = TestFunctions(V) | ||
| f = split(Function(V)) | ||
|
|
||
| for i, j in [(0, 1), (1, 0)]: | ||
| a1 = inner(u[i], v[j])*dx(domain=mesh1) | ||
| with pytest.raises(NotImplementedError): | ||
| assemble(a1) | ||
| a2 = inner(u[i], v[j])*dx(domain=mesh2) | ||
| with pytest.raises(NotImplementedError): | ||
| assemble(a2) | ||
| l1 = inner(f[i], v[j])*dx(domain=mesh1) | ||
| with pytest.raises(NotImplementedError): | ||
| assemble(l1) | ||
| l2 = inner(f[i], v[j])*dx(domain=mesh2) | ||
| with pytest.raises(NotImplementedError): | ||
| assemble(l2) | ||
|
|
||
| for i, j in [(0, 0), (1, 1)]: | ||
| a = inner(u[i], v[j])*dx(domain=mesh1) | ||
| if i == 1: | ||
| with pytest.raises(NotImplementedError): | ||
| assemble(a) | ||
| continue | ||
| A = assemble(a) | ||
| assert A.M.values.shape == (V.dim(), V.dim()) | ||
|
|
||
| a = inner(u[0], v[0])*dx(domain=mesh1) + inner(u[0], v[1])*dx(domain=mesh2) | ||
| with pytest.raises(NotImplementedError): | ||
| assemble(a) |
There was a problem hiding this comment.
Most of the integrals here are illegal as intersect_measures is not set. I suppose these tests are about if meshA and meshB are related in the sense of submesh if they appear in a single integral expression, and not about legality of the integral expression, so you first need to write legal integrals, and test if they still raise exception when participating meshes are unrelated. Given that you removed the lines in assemble.py, I imagine that exception is raised when composing maps.
Note that illegality of the integrals was overlooked due to #4775.
There was a problem hiding this comment.
I think this is something different. These are topologically unrelated. These are just testing that we are getting the indexing right.
There was a problem hiding this comment.
All this PR does is removing the guard in assemble.py, so we should be testing the consequence of that. That uncoupled mixed domain problems work is tested in test_multi_domain_solve(), which is fine, but here we should definitely be testing that other forms that would have hit the removed guard still fail correctly, too. I expected that that is what test_multi_domain_assemble() should be doing, and the forms that would have hit the removed guard are those symbolically correct ones defined with intersect_measures that contain unrelated topologies. Those are what the removed guard was for at least.
The illegal forms currently tested in test_multi_domain_assemble() are useful, but I think the above is more relevant to the specific changes made in this PR. In those forms, integral_types are not defined for some participating meshes, so they must cause exception earlier at more fundamental level (TSFC) if the topology guard in assemble.py exists or not. integral_type not defined on some mesh is a broader problem.
I don't remember the details, but those illegal forms must have caused some exception in TSFC as expected before #4775 was merged, but #4775 partially broke it, and this PR added some guard in TSFC (for Arguments) on top of that. So the change made in TSFC in this PR might not be necessary once #4775 is reverted (except that the exception raised in TSFC could probably be clearer).
| for arg in arguments: | ||
| if domain_integral_type_map[extract_unique_domain(arg)] is None: | ||
| raise NotImplementedError("Assembly of forms over unrelated meshes is not supported. " | ||
| "Try using Submeshes or cross-mesh interpolation.") | ||
|
|
There was a problem hiding this comment.
This is checking legality of the integral, so the error message looks incorrect. Given a legal integral, topological check must happen in firedrake (not in tsfc). As mentioned in the above, exception should be raised when composing maps, but this needs to be checked.
allow assembly of uncoupled multi-domain problems