ufc::element::evaluate_basis_derivatives_all broken for mixed elements

Issue #13 invalid
Prof Garth Wells created an issue

The generated code body for ufc::element::evaluate_basis_derivatives_all exceeds array bounds. For a 3D P1 elasticity problem, it generates:

// Loop dofs and call evaluate_basis_derivatives.
for (unsigned int r = 0; r < 12; r++)
{
  evaluate_basis_derivatives(r, n, dof_values, x, vertex_coordinates, cell_orientation);
  for (unsigned int s = 0; s < 3*num_derivatives; s++)
  {
    values[r*3*num_derivatives + s] = dof_values[s];
  }// end loop over 's'
}// end loop over 'r'

For first order derivatives, num_derivatives=3 (x, y, z). Hence, the maximum index to values is 117. However, the number of basis function derivatives is 36 (433).

Comments (14)

  1. Kristian Ølgaard

    Yes, that's what I usually do, but the Edit button seems to be missing. Only Attach is available (and the Create issue link)

  2. Kristian Ølgaard

    If you look at a vector p1 element it has 12 dofs each with 3 values (value_rank is 1 and value_dimension is 3) this is the r3 part of the code. So I think the maximum index will be 1133+8=107 (108 values 123*3). It looks correct to me.

    If for instance you call evaluate_basis on dof number 0 (with a coordinate equal to where dof 0 is located), it will return [1, 0, 0], so a call to evaluate_basis_derivatives will return 9 values instead.

  3. Prof Garth Wells reporter

    Lagrange basis functions are not vectors spaces. Each dof has one value.

    A P1 Lagrange vector element in 3D (on a tetrahedron) has 12 dofs, each with 1 value. Therefore, number of derivatives is 12*3 (3 for derivatives w.r.t x, y and z).

  4. Kristian Ølgaard

    I suppose that a VectorLagrange function is a vector space? If not, then a lot of things need to be changed inside FFC and the 'error' lies somewhere else than evaluate_basis_derivatives.

    At the moment:

    element = VectorElement("Lagrange", tetrahedron, 1) element = FiniteElement("BDM", tetrahedron, 1)

    are very similar in terms of ufl_element information which forms the basis of the code generation.

    It is possible to generate the code based on the 'number of components' for each individual dof, in which case the return values will have dimension 1 for a call to evaluate_basis on P1 Lagrange and dimension 3 for a call to BDM elements (in 3D).

    However, for a MixedElement([P1, BDM]), which will have 4 + 12 = 16 dofs, evaluate_basis will currently return 4 values for any of the dofs, while with the new approach the function will return 1 value when evaluating the first 4 dofs and 3 values when evaluating the last 12 dofs. So a user must know to which subelement a given dofnumber belongs when calling the function. In this case, it will be much faster to simply call evaluate_basis directly on the subelement of interest.

    So in your case, just call evaluate_basis_derivatives on the subelement (P1 Lagrange).

  5. Prof Garth Wells reporter

    VectorLagrange is a shortcut for a mixed element of Lagrange elements.

    It doesn't make sense that there are more than 12*3 derivatives of 12 scalar functions in 3D.

  6. Martin Sandve Alnæs

    Looks correct to me. This is how the evaluate functions are defined: value components for each sub element are included even if they are always zero. As KBO says, use the sub element to get the scalar values. Otherwise BDM and VectorLagrange could not be handled uniformly.

  7. Johannes Ring

    Kristian: I have added you to the developers list for FFC (you were only in the contributors list), so you should be able to assign yourself to this issue now.

  8. Martin Sandve Alnæs

    So there are 12 * 3 * 3 derivative values of 12 functions with 3 components in 3D.

    FFC implements the functions as specified. You may want or need something else, but that's a feature request and not a bug report.

  9. Kristian Ølgaard

    In principle, I agree with Garth, but as I recall it we (Anders, Marie, Martin and I) discussed this when the functions were implemented. I actually think the first implementation did what Garth wants but it was changed to handle BDM elements in a uniform way like Martin says.

  10. Prof Garth Wells reporter

    Some documentation would be helpful. The UFC documentation of basis function evaluation leaves something to be desired.

  11. Log in to comment