Interior bcs #2007

wence- · 2021-03-12T21:10:30Z

Remove support for method="geometric" boundary conditions and then augment the bc construction such that we can handle bcs on interior marked facets.

@ScottMacLachlan does this work for your use cases? I have only proved it correct, not tested.

firedrake/functionspacedata.py

sv2518 · 2021-04-07T10:39:42Z

I think we should change the docs on "Boundary conditions in discontinuous spaces", where we advise to use geometric boundary conditions, within this PR as well. It's line 568 in https://github.com/firedrakeproject/firedrake/blob/wence/feature/no-geom-bcs/docs/source/variational-problems.rst.

ReubenHill · 2021-04-09T14:42:27Z

I see I've been asked to review - I don't actually know much about how boundary conditions work in firedrake so don't quite feel I am able to comment usefully at the moment. Perhaps if you were to make Sophia's suggested change I might be able to start somewhere with the review?

It's also now quite clear to me why this change as been made in either the description of the PR or the commit that makes the depreciation. Could you please add some context?

wence- · 2021-04-14T15:58:38Z

@ScottMacLachlan have you had a chance to look at this? It would even be useful to have a testcase that you've been using.

ScottMacLachlan · 2021-04-16T13:41:43Z

I now get an error in my call to MeshHierarchy, on a mesh read in from gmesh:

  File "simpleTestSandwich.py", line 52, in <module>
    mh = MeshHierarchy(base, 1)
  File "/gpfs/fs0/scratch/s/scottmac/scottmac/April_2021/firedrake/src/firedrake/firedrake/mg/mesh.py", line 140, in MeshHierarchy
    meshes = [mesh] + [mesh_builder(dm, dim=mesh.ufl_cell().geometric_dimension(),
  File "/gpfs/fs0/scratch/s/scottmac/scottmac/April_2021/firedrake/src/firedrake/firedrake/mg/mesh.py", line 140, in <listcomp>
    meshes = [mesh] + [mesh_builder(dm, dim=mesh.ufl_cell().geometric_dimension(),
  File "/gpfs/fs0/scratch/s/scottmac/scottmac/April_2021/firedrake/lib/python3.8/site-packages/decorator.py", line 231, in fun
    return caller(func, *(extras + args), **kw)
  File "/gpfs/fs0/scratch/s/scottmac/scottmac/April_2021/firedrake/src/PyOP2/pyop2/profiling.py", line 60, in wrapper
    return f(*args, **kwargs)
  File "/gpfs/fs0/scratch/s/scottmac/scottmac/April_2021/firedrake/src/firedrake/firedrake/mesh.py", line 1711, in Mesh
    topology = MeshTopology(plex, name=name, reorder=reorder,
  File "/gpfs/fs0/scratch/s/scottmac/scottmac/April_2021/firedrake/lib/python3.8/site-packages/decorator.py", line 231, in fun
    return caller(func, *(extras + args), **kw)
  File "/gpfs/fs0/scratch/s/scottmac/scottmac/April_2021/firedrake/src/PyOP2/pyop2/profiling.py", line 60, in wrapper
    return f(*args, **kwargs)
  File "/gpfs/fs0/scratch/s/scottmac/scottmac/April_2021/firedrake/src/firedrake/firedrake/mesh.py", line 700, in __init__
    dmcommon.label_facets(plex, label_boundary=label_boundary)
  File "firedrake/cython/dmcommon.pyx", line 994, in firedrake.cython.dmcommon.label_facets
RuntimeError: 63

Any ideas?

wence- · 2021-04-16T13:44:27Z

Can you provide the .geo?

ScottMacLachlan · 2021-04-16T13:46:40Z

Can you provide the .geo?

Here it is (extension changed to allow attachment)...
sandwichUniform_h=005_geo.txt

wence- · 2021-04-16T14:02:16Z

This works forr me. I think that you didn't rebuild the Cython extensions in Firedrake after checking out the branch? If you have a separate PETSc, run make in the firedrake source directory, otherwise run firedrake-update after selecting this branch.

ScottMacLachlan · 2021-04-16T14:16:21Z

You're right (of course). After running make, I get identical output for my test problem using DirichletBC to what I got using the old InteriorBC workaround. Thanks!

ScottMacLachlan · 2021-04-16T14:19:39Z

FWIW, most of the code changes are in places where I'm not qualified to judge correctness. What I do understand, however, looks good to me...

Provide DeprecationWarning in the top-level constructors.

PETSc errors if a label contains points that are not in the index range. This is preparation for "completing" facet labels such that they mark everything in their closure.

We are going to "complete" labels to contain points in their closure, so we must filter the label stratum by points that fall in a given mesh stratum (for example only facet points).

Should no longer be necessary.

This will be used for grabbing DirichletBC nodes from section data.

The magic method forwarding doesn't work in this case because we really want to pass "self" into the shared data boundary_node extraction (not self.topological).

Now that we're only handling topological boundary conditions, we can just inspect the section of the function space to determine which nodes to mask out. This also enables us to easily support constraining dofs that live on interior facets, so do that too. Fixes #1661, #1135.

wence- · 2021-05-05T11:03:18Z

I think we should change the docs on "Boundary conditions in discontinuous spaces", where we advise to use geometric boundary conditions, within this PR as well. It's line 568 in https://github.com/firedrakeproject/firedrake/blob/wence/feature/no-geom-bcs/docs/source/variational-problems.rst.

I did this, thanks.

sv2518

LGTM

wence- · 2021-05-06T17:20:15Z

Added a very simple test @ScottMacLachlan do you want to sanity-check?

I also updated the triplot function to colour interior mesh markers as well, @danshapero does that look sane?

ScottMacLachlan

Looks good to me, and works for my test case

danshapero · 2021-05-12T16:29:29Z

The plotting changes look good. I also tried it out with an unstructured quad mesh and everything looked fine there too.

wence- mentioned this pull request Mar 12, 2021

Enable interior boundary conditions #1661

Closed

wence- commented Mar 12, 2021

View reviewed changes

firedrake/functionspacedata.py Outdated Show resolved Hide resolved

wence- force-pushed the wence/feature/no-geom-bcs branch 3 times, most recently from 683271f to d5c2a87 Compare March 15, 2021 12:24

wence- requested review from ScottMacLachlan and ReubenHill March 15, 2021 18:33

wence- force-pushed the wence/feature/no-geom-bcs branch from d5c2a87 to f993f57 Compare April 19, 2021 09:31

wence- added 10 commits May 5, 2021 10:26

bcs: Expunge method="geometric"

b71de69

Provide DeprecationWarning in the top-level constructors.

mesh: Remove boundary_faces label after use

e5b1ae6

mesh: Always create label indices over full chart

b435243

PETSc errors if a label contains points that are not in the index range. This is preparation for "completing" facet labels such that they mark everything in their closure.

cython: Filter label stratum sizes by point strata

feb09f6

We are going to "complete" labels to contain points in their closure, so we must filter the label stratum by points that fall in a given mesh stratum (for example only facet points).

mg: Remove label filtering

88a443c

Should no longer be necessary.

mesh: Complete facet labels after adding overlaps

d707967

This will be used for grabbing DirichletBC nodes from section data.

functionspace: boundary_node on WithGeometry

641fdf3

The magic method forwarding doesn't work in this case because we really want to pass "self" into the shared data boundary_node extraction (not self.topological).

docs: Update description of boundary conditions

73ba1bb

Ignore generated file

ee86830

wence- force-pushed the wence/feature/no-geom-bcs branch from f993f57 to ee86830 Compare May 5, 2021 09:43

sv2518 previously approved these changes May 5, 2021

View reviewed changes

wence- added 2 commits May 6, 2021 18:06

plot: Colour interior facet markers too

0a804c2

Add test of interior boundary conditions

fc6bc46

wence- dismissed sv2518’s stale review via fc6bc46 May 6, 2021 17:18

ScottMacLachlan approved these changes May 6, 2021

View reviewed changes

danshapero approved these changes May 12, 2021

View reviewed changes

wence- merged commit 31b3c11 into master May 13, 2021

wence- deleted the wence/feature/no-geom-bcs branch May 13, 2021 13:38

tkarna mentioned this pull request May 17, 2021

remove geometric boundary mask thetisproject/thetis#241

Merged

This was referenced May 18, 2021

Multiple boundary tags in meshmode export inducer/meshmode#186

Open

Complete boundary labels on topology once #1135

Closed

tkarna mentioned this pull request May 28, 2021

2d depth-integrated non-hydrostatic model thetisproject/thetis#225

Merged

ScottMacLachlan mentioned this pull request Apr 18, 2022

How to cancel the warning #2415

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Interior bcs #2007

Interior bcs #2007

wence- commented Mar 12, 2021

sv2518 commented Apr 7, 2021

ReubenHill commented Apr 9, 2021

wence- commented Apr 14, 2021

ScottMacLachlan commented Apr 16, 2021

wence- commented Apr 16, 2021

ScottMacLachlan commented Apr 16, 2021

wence- commented Apr 16, 2021

ScottMacLachlan commented Apr 16, 2021

ScottMacLachlan commented Apr 16, 2021

wence- commented May 5, 2021

sv2518 left a comment

wence- commented May 6, 2021

ScottMacLachlan left a comment

danshapero commented May 12, 2021

Interior bcs #2007

Interior bcs #2007

Conversation

wence- commented Mar 12, 2021

sv2518 commented Apr 7, 2021

ReubenHill commented Apr 9, 2021

wence- commented Apr 14, 2021

ScottMacLachlan commented Apr 16, 2021

wence- commented Apr 16, 2021

ScottMacLachlan commented Apr 16, 2021

wence- commented Apr 16, 2021

ScottMacLachlan commented Apr 16, 2021

ScottMacLachlan commented Apr 16, 2021

wence- commented May 5, 2021

sv2518 left a comment

Choose a reason for hiding this comment

wence- commented May 6, 2021

ScottMacLachlan left a comment

Choose a reason for hiding this comment

danshapero commented May 12, 2021