Dualspace update #2294
Dualspace update #2294
Conversation
|
This is ready to go once UFL upstream is merged. |
| @@ -139,7 +137,7 @@ def evaluate_tlm_component(self, inputs, tlm_inputs, block_variable, idx, | |||
| dudmi.assign(ufl.algorithms.expand_derivatives( | |||
| ufl.derivative(expr, dep.saved_output, | |||
| dep.tlm_value))) | |||
| dudm.vector().axpy(1.0, dudmi.vector()) | |||
| dudm.dat += 1.0 * dudmi.dat | |||
There was a problem hiding this comment.
I've never seen direct operations on a PyOP2.dat like this before. @connorjward is it the right thing to be doing?
There was a problem hiding this comment.
This is exactly what Vector.axpy used to do. We are just lifting the code to not rely on Vector.
| @@ -109,14 +107,14 @@ def _adj_assign_constant(self, adj_output, constant_fs): | |||
| shape = r.ufl_shape | |||
| if shape == () or shape[0] == 1: | |||
| # Scalar Constant | |||
| r.vector()[:] = adj_output.vector().sum() | |||
| r.dat.data[:] = adj_output.dat.data_ro.sum() | |||
There was a problem hiding this comment.
Nitpicking, but this I think should be r.dat.data_wo[:]
There was a problem hiding this comment.
| r.dat.data[:] = adj_output.dat.data_ro.sum() | |
| r.dat.data_wo[:] = adj_output.dat.data_ro.sum() |
There was a problem hiding this comment.
We are modifying the value of r so why should we use dat.data_ro ? .dat.data needs to be used whenever you change the value of the .dat, right ?
There was a problem hiding this comment.
Apologies for the confusion, I meant to say dat.data_wo and hastily edited the comment after making it but clearly it didn't make it to you in time! I think dat.data_wo is correct here since you don't read the value
There was a problem hiding this comment.
Well, Dat.data_wo is the same as Dat.data (see https://github.com/OP2/PyOP2/blame/master/pyop2/types/dat.py#L223). Also, we use Dat.data when changing the value of the Dat all over the code so I don't think that is a problem.
There was a problem hiding this comment.
Well I know we've been trying to move towards using the correct access descriptors wherever we ought to. Whether or not the implementation is different @connorjward can comment on. It's just a small change but I think it's a correct one. If you're really not convinced then let @connorjward weigh in
This PR contains the full dual space implementation. It builds on top of #2034, which introduced dual function spaces, the
CofunctionandAssembledMatrixclasses, as well as the DAG-based assembly ofBaseFormobjects.Main components of this PR:
CoargumentBaseFormbeing assembled)Cofunctionin the rhs (fully annotated operation -> pyadjoint-friendly)ActionandFormSumCofunctions withassignand augmented assignmentsVectoras the default type for adjoint/tlm/hessian valuesriesz_representationto map fromCofunctions toFunctions and vice-versaCofunctions andCoarguments now respectively have 1 and 2 arguments)The associated pyadjoint PR, which simply updates tests, can be found at dolfin-adjoint/pyadjoint#70