hp Version of KellyErrorEstimator<1,spacedim>

[ghost]

The internal error message was a little bit confusing. Looking in the code shows, that this is just not implemented. Would this be straightforward or is that not so easy?

[marcfehling]

Yes, this should be a ExcNotImplemented:

Comparing to the other functions, I can only spot a difference to this overload:

dealii/source/numerics/error_estimator_1d.cc

Lines 165 to 199 in 3d0a9b8

	template <int spacedim>
	template <typename InputVector>
	void
	KellyErrorEstimator<1, spacedim>::estimate(
	const Mapping<1, spacedim> & mapping,
	const DoFHandler<1, spacedim> &dof_handler,
	const hp::QCollection<0> & quadrature,
	const std::map<types::boundary_id,
	const Function<spacedim, typename InputVector::value_type> *>
	& neumann_bc,
	const InputVector & solution,
	Vector<float> & error,
	const ComponentMask & component_mask,
	const Function<spacedim> *coefficients,
	const unsigned int n_threads,
	const types::subdomain_id subdomain_id,
	const types::material_id material_id,
	const Strategy strategy)
	{
	// just pass on to the other function
	const std::vector<const InputVector *> solutions(1, &solution);
	std::vector<Vector<float> *> errors(1, &error);
	estimate(mapping,
	dof_handler,
	quadrature,
	neumann_bc,
	solutions,
	errors,
	component_mask,
	coefficients,
	n_threads,
	subdomain_id,
	material_id,
	strategy);
	}

This function is implemented for just one solution, rather than for a collection. But that should already be implemented, as they convert that one solution into a vector of solutions. Funny.

I assume you somehow ran into this exception in one of your applications. Can you try to call

   estimate(mapping, 
            dof_handler, 
            quadrature, 
            neumann_bc, 
            solutions, 
            errors, 
            component_mask, 
            coefficients, 
            n_threads, 
            subdomain_id, 
            material_id, 
            strategy); 

instead of throwing the exception there and see it works? This way, we can resolve the mystery behind this function :)

[ghost]

@marcfehling thanks for looking into the code! That one solution vector is wrapped by a std::vector<const InputVector *> is not a problem, in fact the problem is that if I have a hp problem the wrong function is called in the end. Tracing back all the wrappers the ultimate call should go to the function, which throws the assert

dealii/source/numerics/error_estimator_1d.cc

Lines 270 to 290 in 8a0eed8

	template <int spacedim>
	template <typename InputVector>
	void
	KellyErrorEstimator<1, spacedim>::estimate(
	const Mapping<1, spacedim> & /*mapping*/,
	const DoFHandler<1, spacedim> & /*dof_handler*/,
	const hp::QCollection<0> &,
	const std::map<types::boundary_id,
	const Function<spacedim, typename InputVector::value_type> *>
	& /*neumann_bc*/,
	const std::vector<const InputVector *> & /*solutions*/,
	std::vector<Vector<float> *> & /*errors*/,
	const ComponentMask & /*component_mask_*/,
	const Function<spacedim> * /*coefficient*/,
	const unsigned int,
	const types::subdomain_id /*subdomain_id*/,
	const types::material_id /*material_id*/,
	const Strategy /*strategy*/)
	{
	Assert(false, ExcInternalError());
	}

However, currently the final goal is this function

dealii/source/numerics/error_estimator_1d.cc

Lines 294 to 311 in 8a0eed8

	template <int spacedim>
	template <typename InputVector>
	void
	KellyErrorEstimator<1, spacedim>::estimate(
	const Mapping<1, spacedim> & mapping,
	const DoFHandler<1, spacedim> &dof_handler,
	const Quadrature<0> &,
	const std::map<types::boundary_id,
	const Function<spacedim, typename InputVector::value_type> *>
	& neumann_bc,
	const std::vector<const InputVector *> &solutions,
	std::vector<Vector<float> *> & errors,
	const ComponentMask & component_mask,
	const Function<spacedim> * coefficient,
	const unsigned int,
	const types::subdomain_id subdomain_id_,
	const types::material_id material_id,
	const Strategy strategy)

Forcing the right function call, by passing a single Quadrature<dim> object even in the hp case did work (it is in fact already implemented as hp vesion). So I would suggest just to move the content of the two mentioned functions, where we replace the assert by another wrapper.

[marcfehling]

Your approach should work since if you are using a single Quadrature object, the conversion constructor for a QCollection is called implicitly.

It looks like the windows CI has some trouble with that.

d:\a\dealii\dealii\source\numerics\error_estimator_1d.cc(559): warning C4717: 'dealii::KellyErrorEstimator<1,1>::estimate<dealii::Vector<float> >': recursive on all control paths, function will cause runtime stack overflow [D:\a\dealii\dealii\build\source\numerics\obj_numerics_debug.vcxproj]

I guess that you need to either get rid of the function you deleted in source/numerics/error_estimator_1d.inst.in as well, or keep the function you deleted and call the internal estimate function.

[ghost]

Your approach should work since if you are using a single Quadrature object, the conversion constructor for a QCollection is called implicitly.

The conversion constructor is not needed as the input argument for the quadrature is not used, see

dealii/source/numerics/error_estimator_1d.cc

Line 300 in 8a0eed8

const Quadrature<0> &,

. So it is just to trigger the hp version of this function. In fact, I copied the content of the corresponding function and replaced the non-hp version as a wrapper.

It looks like the windows CI has some trouble with that.

d:\a\dealii\dealii\source\numerics\error_estimator_1d.cc(559): warning C4717: 'dealii::KellyErrorEstimator<1,1>::estimate<dealii::Vector<float> >': recursive on all control paths, function will cause runtime stack overflow [D:\a\dealii\dealii\build\source\numerics\obj_numerics_debug.vcxproj]

I guess that you need to either get rid of the function you deleted in source/numerics/error_estimator_1d.inst.in as well, or keep the function you deleted and call the internal estimate function.

For the warning I have to check the inst file ... I just wonder, because I did not delete any function, just switched the content.

In addition I used the PR to reorder the functions in the source file according to the occurence in the header.

[ghost]

Has someone seen this error in the tester once. I dont understand it right now.

[marcfehling]

I just wonder, because I did not delete any function, just switched the content.

I didn't read your patch correctly at first. You are right, you just changed the input parameters.

Has someone seen this error in the tester once. I dont understand it right now.

I think with your change you accidentally caused one of the overloads to call itself, which will end up in a infinite recursion loop.

[ghost]

Has someone seen this error in the tester once. I dont understand it right now.

I think with your change you accidentally caused one of the overloads to call itself, which will end up in a infinite recursion loop.

OK, thanks, so I will take a second look. But strange, the linux testers are happy...

[ghost]

Has someone seen this error in the tester once. I dont understand it right now.

I think with your change you accidentally caused one of the overloads to call itself, which will end up in a infinite recursion loop.

OK, thanks, so I will take a second look. But strange, the linux testers are happy...

Should now be fixed...

[marcfehling]

/rebuild

[marcfehling]

Looks good to me.

However, your change of reordering the functions makes this patch horrible to review now :-)
I would like to wait for a second approval just in case I missed something.

[marcfehling]

Well, I compared both master and this feature branch another time and I don't find anything missing.

@gfcas -- I think a changelog entry would be nice. I would merge the PR after you provided one :)

[ghost]

Looks good to me.

However, your change of reordering the functions makes this patch horrible to review now :-) I would like to wait for a second approval just in case I missed something.

I totally agree with you, sorry ... I reverted the reordering of the functions and added a minor(?) changelog entry. Thanks for reviewing!