[WIP] Modularize assembly

[bangerth]

This is my second try, following #695.

The idea here is to introduce a structure that contains signals that point to functions that do the actual assembly loop. The setup of scratch objects etc continues to happen at an outer level, but on every cell we then simply call each slot in the signal, and thereby add up terms as appropriate for an equation or approximation.

The current patch is simply a strawman that just does this for the Stokes preconditioner and assembly. It seems to pass the first few tests on my machine, but that's not the point right now.

Points to discuss:

• Right now, the assembler functions are free functions in a namespace. This makes it relatively easy to add other, similar implementations. It has the disadvantage that the functions do not readily have access to member variables, and so one has to pass a SimulatorAccess object to them. I worry that if we create more complex assemblers, that they need more than what they get right now. On the other hand, this probably encourages a clean design where we use as little information as we can. What are people's opinions on free functions? I could as well declare a class CompleteEquations (rather than a namespace) which we could then derive from SimulatorAccess. I'm not really sure we'd save very much this way, though.
• The signals are declared with only those arguments that differ from invocation to invocation. The functions have other arguments such as the SimulatorAccess or Introspection argument. I simply std::bind them in set_assemblers(). This style allows me to define these functions with interfaces that differ by additional const arguments that are known at the time set_assemblers() is run.
• Completion of the conversion by doing the same for the temperature/advection system assembly as well as residual assembly, remains to be done. I wanted to have something out for discussion first, before doing the entire conversion.

[tjhei]

is there a reason you go for SimulatorAccess for the Stokes system but Introspection for the preconditioner? I would prefer an identical interface.

[bangerth]

Fair enough. My guiding line was to ask for as little as I could get away with, but a SimulatorAccess object makes things uniform. Will come up in the next patch iteration.

[tjhei]

This looks really nice!

Right now, the assembler functions are free functions in a namespace.

I see a point in the future where the simulator might be interested in querying certain information from each assembler, for example if they need certain information precomputed. I am thinking of asking each assembler which AdditionalOutputs object they need (this needs to happen before the material model is evaluated). Then it makes more sense to have instances of classes. Maybe we should go there immediately?

The signals are declared with only those arguments that differ from invocation to invocation.

I would prefer similar interfaces. In fact, if we get rid of the separate scratch object for the preconditioner (is that really necessary?), preconditioner assembly and system assembly can be identical.

Completion of the conversion by doing the same for the temperature/advection system assembly as well as residual assembly, remains to be done.

I am happy to leave that for a separate PR. There is assembly for the advection residual for entropy viscosity and we could also introduce a function for the non-linear residual (or are they identical?).

I think it would be great to have this merged soon. We can always rearrange things into different files later.

@jdannberg have you started playing with this for the melt transport? Let us know how that goes.

[bangerth]

Right now, the assembler functions are free functions in a namespace.

I see a point in the future where the simulator might be interested in
querying certain information from each assembler, for example if they need
certain information precomputed. I am thinking of asking each assembler which
AdditionalOutputs object they need (this needs to happen before the material
model is evaluated). Then it makes more sense to have instances of classes.
Maybe we should go there immediately?

I've given this a bit of thought over the past two days. Putting everything
into a class has the advantage that we can derive it from SimulatorAccess,
and that means we won't have to pass such an object to every function via
std::bind. We could also introduce a common set of virtual functions.

On the other hand, I think that the design will not allow the assemblers
themselves to be virtual functions since they just need a different set of
inputs. Take, for example, the functions that compute the Stokes system and
the function that computes the pressure_rhs_compatible vectors. They're just
too different.

For similar reasons, I'm doubtful that we will gain much by inheriting one
assembler class from another. I think we're just going to see a complete
tangle from deriving one assembler class from another.

Finally, if I put everything into classes, I find it difficult to mix and
match terms. Right now, it's all just individual functions that I can choose
and pick at will.

Let me try something here.

The signals are declared with only those arguments that differ from
invocation to invocation.

I would prefer similar interfaces. In fact, if we get rid of the separate
scratch object for the preconditioner (is that really necessary?),
preconditioner assembly and system assembly can be identical.

I don't see that working. The assemblers just do things differently enough
that I can't see us getting them into the same interface.

Completion of the conversion by doing the same for the
temperature/advection system assembly as well as residual assembly,
remains to be done.

I am happy to leave that for a separate PR. There is assembly for the
advection residual for entropy viscosity and we could also introduce a
function for the non-linear residual (or are they identical?).

I think it would be great to have this merged soon. We can always rearrange
things into different files later.
OK.

[bangerth]

@tjhei -- I've updates the first commit with your smaller comments. The second commit restructures things so that for the complete equation, the assemblers are now wrapped in a class. This design does not require functions to be part of classes, but they are allowed to if it makes their design simpler. Do you like this better?

[bangerth]

I've pushed two more commits that now also convert assembly of the traction boundary condition and free surface terms.

[jdannberg]

@tjhei @bangerth
I have tried to fit our melt assembly into the new structure, and one problem that I found was that in our local_assemble_stokes_system we also have a surface integral, and we need to recompute all of the material properties on the faces, so we need for example compute_material_model_input_values, which is a private member of the Simulator class, so we can not access it from inside the assembler.

I put everything I have so far here: https://github.com/jdannberg/aspect/tree/melt_assembly
(it does not compile, the part I mentioned above is in assembly.cc around line 1422)

[tjhei]

It is probably okay to put something like compute_material_model_input_values into simulatoraccess. Alternatively, we could have a separate function for face assembly and the material model evaluation, etc. then happens in the generic loop (and only once!).

[jdannberg]

Yes, I like having a separate function for the face assembly. It would also make the melt assembler much shorter. And we need the current_linearization_point in compute_material_model_input_values too, so that would be another thing we would have to add to simulatoraccess otherwise.

[bangerth]

On 01/17/2016 11:46 PM, Juliane Dannberg wrote:

@tjhei https://github.com/tjhei @bangerth https://github.com/bangerth
I have tried to fit our melt assembly into the new structure, and one problem
that I found was that in our |local_assemble_stokes_system| we also have a
surface integral, and we need to recompute all of the material properties on
the faces, so we need for example |compute_material_model_input_values|, which
is a private member of the Simulator class, so we can not access it from
inside the assembler.

I put everything I have so far here:
https://github.com/jdannberg/aspect/tree/melt_assembly
(it does not compile, the part I mentioned above is in |assembly.cc| around
line 1422)

I think you should put the melt terms into their own class, rather than put
them in with the CompleteEquations term. You could forward declare this class
at the top of simulator.h and make it a friend of the Simulator class if you
want. Or do as @tjhei suggests and make the function available via the
SimulatorAccess interface.

[bangerth]

What's people's opinion on the current state of the patch?

[tjhei]

would it make sense to include the current cell also for the stokes preconditioner?

[bangerth]

I chose the minimal interface that works. All of these are internal interfaces and I suspect that we will adjust them as we use them more.

[tjhei]

What's people's opinion on the current state of the patch?

I have some smaller nitpicks, but I think we can move forward and merge it and then evolve the interface as we go.

[bangerth]

On 01/19/2016 07:46 AM, Timo Heister wrote:

I have some smaller nitpicks, but I think we can move forward and merge
it and then evolve the interface as we go.

Well, let me know what you want me to change.

[tjhei]

I just hit "merge".

Well, let me know what you want me to change.

• unify the interface
• extend to temperature/composition
• separate functions for face assembly (function call for each face)

[bangerth]

Yes, @jdannberg and @gassmoeller and I just talked about the face assembly. I have a plan.

Will do temperature/composition after that.

[bangerth]

In reference to #733.