Function::eval() and Function::non_matching_eval() are identical

[Chris Richardson]

void Function::eval(Array<double>& values, const Array<double>& x) const

and

void Function::non_matching_eval(Array<double>& values, const Array<double>& x, const ufc::cell& ufc_cell) const

are apparently identical. Maybe I am missing something, but can one of them be deleted?

[Martin Sandve Alnæs]

@logg @august may have opinions

[Chris Richardson]

I'd like to clean up Function evaluation, if possible. At the moment there is a very long chain of function calls e.g. from Function::iterpolate to Function::eval(values, x, cell, ufc_cell). e.g. Issue #507 is hard to fix at the moment, because of this complexity.

[Prof Garth Wells]

@chris_richardson Yes, the logic around eval is complicated, with the flow going in and out of the UFC layer.

The solution is probably tighter integration between UFC and DOLFIN. Since UFC is only used by DOLFIN, I think we all agreed that tighter integration is ok.

[Martin Sandve Alnæs]

Depends on what you mean by 'tighter integration'. We all agree that changes to the ufc interface leading to leaner dolfin code would be a good thing, and we do not need to consider hypothetical external users of ufc. But I still think it's worthwhile to keep ufc (i.e. the interfaces implemented by generated code) independent in the sense that they do not include headers from dolfin. The reason is that developers must know that when these are changed, the code generation must be updated as well.

[Prof Garth Wells]

I mean that we do not need to consider hypothetical external users of UFC.

I think we should discuss the inclusion of DOLFIN data types in the generated code if it leads to significant simplifications. One candidate is dolfin::Cell in place of ufc::cell. It would be very nice if we could straighten the code path from dolfin::Function::eval by changing UFC; this will require a careful look at how it currently works, which isn't pretty. We should throw into the mix dolfin::Function::eval for multiple cells to avoid the overhead of many calls to the linear algebra backend to fetch little data each time, which for some problems is the dominant cost in assembly.

[Martin Sandve Alnæs]

We agree that we do not need to consider hypothetical external users of UFC, and I think the changes we've done to ufc over the last couple of years show that.

If ufc::cell in the ufc::function interface is the main problem here, the ufc::function interface can be changed at will from a ufc/ffc point of view to make dolfin cleaner.

However if ufc::function is made to depend on dolfin::Cell, either dolfin::Cell must be moved to become the new ufc::cell with no dolfin dependencies, or that would introduce a dolfin.h dependency from ufc.h which is not nice.

I think constructive solutions going forward depend on pinpointing exactly what we need to solve here. Maybe ufc::function can get raw data instead of the cell object, like we did with the cell argument to tabulate_tensor?

[Martin Sandve Alnæs]

As a sidenote: on the issue stack, perhaps for 1.7, is the introduction of functions callable from generated code in quadrature points. For that we will also need something better than the current ufc::function.

[Prof Garth Wells]

@martinal Are you referring to the issue https://bitbucket.org/fenics-project/dolfin/issue/355/?

[Chris Richardson]

Maybe we can split off ufc interface improvements as a separate issue? I thought I could just start an incremental clean up of function evaluation, starting by removing the duplicated methods... if everyone is OK with that?

[Martin Sandve Alnæs]

@garth-wells yes

@chris_richardson maybe keep the duplicate method (non_matching_eval) as non-virtual just calling deprecated + the remaining method (eval)?

Iterative clean up will hopefully make it clearer what we need in the end so I'm all for that.

[Chris Richardson]

Merged