Dealii: Invalid manifold id is the same as flat manifold id

[jppelteret]

Looking at our choice of IDs for invalid and flat manifolds, they are currently set to the same value. I'm guessing that this is unintentional, and that what we really want is

const types::manifold_id flat_manifold_id = static_cast<types::manifold_id>(-2);

or

const types::manifold_id flat_manifold_id = static_cast<types::manifold_id>(invalid_manifold_id-1);

(mimicing the choice of artificial_subdomain_id and invalid_subdomain_id).
@luca-heltai what do you think?

[luca-heltai]

I think there should be no such a thing as an "invalid manifold id". We should remove it from everywhere, and just leave "flat manifold id".

[jppelteret]

There are 35 references to it, most of which can be readily changed to flat_manifold_id. Here's the one's that I'm concerned about:

1. Documentation in boundary.h
2. Invalid accessor

template <int structdim, int dim, int spacedim>
types::manifold_id
InvalidAccessor<structdim, dim, spacedim>::manifold_id () const
{
  return numbers::invalid_manifold_id;
}

1. Assertion 1

template <int dim, int spacedim>
void
Triangulation<dim, spacedim>::set_manifold (const types::manifold_id m_number,
                                            const Manifold<dim, spacedim> &manifold_object)
{
  Assert(m_number < numbers::invalid_manifold_id,
         ExcIndexRange(m_number,0,numbers::invalid_manifold_id));

  manifold[m_number] = manifold_object.clone();
}

1. Assertion 2

template<int dim, int spacedim>
void Triangulation<dim, spacedim>::reset_manifold(const types::manifold_id m_number)
{
  Assert(m_number < numbers::invalid_manifold_id,
         ExcIndexRange(m_number,0,numbers::invalid_manifold_id));

  //delete the entry located at number.
  manifold.erase(m_number);
}

With these in mind, should I still go ahead and remove invalid_manifold_id?

[luca-heltai]

Documentation in boundary.h

This documentation is no longer updated, and needs to be changed. It is no longer true that the boundary_id is queried if invalid_manifold_id is on the boundary.

This one:

Invalid accessor
template <int structdim, int dim, int spacedim>
types::manifold_id
InvalidAccessor<structdim, dim, spacedim>::manifold_id () const
{
  return numbers::invalid_manifold_id;
}

is actually ok even if you replace it by numbers::flat_manifold_id.

Assertion 1
template <int dim, int spacedim>
void
Triangulation<dim, spacedim>::set_manifold (const types::manifold_id m_number,
                                            const Manifold<dim, spacedim> &manifold_object)
{
  Assert(m_number < numbers::invalid_manifold_id,
         ExcIndexRange(m_number,0,numbers::invalid_manifold_id));

  manifold[m_number] = manifold_object.clone();
}

Assertion 2
template<int dim, int spacedim>
void Triangulation<dim, spacedim>::reset_manifold(const types::manifold_id m_number)
{
  Assert(m_number < numbers::invalid_manifold_id,
         ExcIndexRange(m_number,0,numbers::invalid_manifold_id));

  //delete the entry located at number.
  manifold.erase(m_number);
}

same here. This all makes sense if you replace with flat_manifold_id.

[luca-heltai]

I vote for deprecating invalid_manifold_id, and making sure that in the library + tests we only use flat_manifold_id.

[jppelteret]

I vote for deprecating invalid_manifold_id, and making sure that in the library + tests we only use flat_manifold_id.

That was going to be my next question: Remove vs deprecate. I'm going to hold off for a day to let the others weigh in on this before I open a PR for this. Thanks for your input @luca-heltai!

[tjhei]

I vote for deprecating invalid_manifold_id

It might make sense to remove it outright. It is only used in two tests (./mpi/fe_field_function_02.cc and ./fail/arclength_boundary_02.cc) and shouldn't be used in user code. If it is, it is likely something the user need to check regardless.

[bangerth]

On 04/30/2018 08:34 AM, Jean-Paul Pelteret wrote:

1. https://github.com/dealii/dealii/blob/master/doc/doxygen/headers/boundary.h?utf8=%E2%9C%93#L114-L134
2. Invalid accessor
   https://github.com/dealii/dealii/blob/master/include/deal.II/grid/tria_accessor.templates.h?utf8=%E2%9C%93#L520

template
types::manifold_id
InvalidAccessor::manifold_id ()const
{
return numbers::invalid_manifold_id;
}

I believe that creating such an iterator object throws an exception anyway. In
other words, you cannot get to this place, the function only exists to make
the linker happy.