-
Notifications
You must be signed in to change notification settings - Fork 174
Real function space #909
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
Real function space #909
Conversation
This must be a hole in our test coverage.
Conflicts: firedrake/ffc_interface.py firedrake/solving.py
Conflicts: .travis.yml
| def exterior_facet_boundary_node_map(self, method): | ||
| """":class:`RealFunctionSpace` objects have no exterior facet boundary | ||
| node map.""" | ||
| return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To allow field splitting to work with real spaces (especially inside bigger problems) we need to enable the .dm property correctly. And also the ._ises property. This will need to be special-cased. So you should implement the _dm method that makes this DM. The way I do it is that PyOP2 supplies the DM and the field_ises via the dof_dset of the function space. This is presumably not implemented for GlobalDataSet objects.
| """:class:`RealFunctionSpace` objects have no node set.""" | ||
| self.node_set = None | ||
| """:class:`RealFunctionSpace` objects have no dof set.""" | ||
| self.dof_dset = op2.GlobalDataSet(self.make_dat()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I make two RealFunctionSpace objects, they currently don't share a dof_dset. This may be a problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not clear to me, in the pyop2 implementation, why a global dataset needs to remember the global used to make it (and not just the shape, etc...).
| # Error checking | ||
| for space in spaces: | ||
| if type(space) is impl.FunctionSpace: | ||
| if type(space) in (impl.FunctionSpace, impl.RealFunctionSpace): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why, when RealFunctionSpace inherits FunctionSpace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type, not isinstance.
Conflicts: firedrake/assemble.py
Codecov Report
@@ Coverage Diff @@
## master #909 +/- ##
=========================================
Coverage ? 89.02%
=========================================
Files ? 52
Lines ? 6615
Branches ? 931
=========================================
Hits ? 5889
Misses ? 569
Partials ? 157 |
Conflicts: .drone.yml .travis.yml
Conflicts: .drone.yml
When the row-space of a 2-form is in R, we were still using index 1 for the columns, really it should be 0.
.travis.yml
Outdated
| - mkdir tmp | ||
| - cd tmp | ||
| - ../scripts/firedrake-install --disable-ssh --minimal-petsc ${SLEPC} --adjoint --slope --install thetis --install gusto ${PACKAGE_MANAGER} | ||
| - ../scripts/firedrake-install --disable-ssh --minimal-petsc ${SLEPC} --adjoint --slope --install thetis --install gusto ${PACKAGE_MANAGER} --package-branch PyOP2 globals_petsc_changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This branch is already merged, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this is .travis.yml...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes needed in the definition of RealFunctionSpace, but otherwise fine I think.
firedrake/functionspaceimpl.py
Outdated
| def __init__(self, mesh, element, name): | ||
|
|
||
| self.name = name | ||
| self._index = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary attribute.
firedrake/functionspaceimpl.py
Outdated
| self.comm = mesh.comm | ||
|
|
||
| """:class:`RealFunctionSpace` objects have no node set.""" | ||
| self.node_set = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of these are argument independent, so I would make them class attributes:
class RealFunctionSpace(FunctionSpace):
finat_element = None
dim = 1
rank = 0
shape = ()
node_set = None
def __init__(self, mesh, element, name):
self._ufl_element = element
self.name = name
self.comm = mesh.comm
self._mesh = mesh
self.dof_dset = op2.GlobalDataSet(self.make_dat())Note as well we now have finat_element, not fiat_element.
firedrake/tsfc_interface.py
Outdated
|
|
||
| a = form.arguments() | ||
| reals = map(lambda x: x.ufl_element().family() == "Real", a) | ||
| if not a or not any(reals): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the second condition is subsumed by the first, since any(()) => False.
The long-awaited R-space. Most of the heavy lifting has already gone into PyOP2. This mostly just makes the Firedrake wrappers behave correctly. There is also a new manual section explaining how this works.