MGTwoLevelTransfer: Clean old vector content in prolongate_add

[kronbichler]

This fixes a bug observed by @nfehn in exadg/exadg#453, where old content of a temporary vector would spoil the result. The reason is that we had a separate logic for DG elements that could set vector elements (rather than add-into) at some point, and forgot to clean that up. Our standard tests would not pick this up because we only check the transfer once, whereas it should run twice to expose this bug.

I could observe the bug in the test, returning vectors with entry 2 rather than 1 in some of the outputs, so this is a sensitive test for the bug I'm fixing.

[peterrum]

@kronbichler Thank you for debugging this.

The reason is that we had a separate logic for DG elements that could set vector elements (rather than add-into) at some point, and forgot to clean that up.

We made the change here: eef0a63

What about reintroducing the logic? The right choice would be to use VectorSetter here

dealii/include/deal.II/multigrid/mg_transfer_global_coarsening.templates.h

Lines 2756 to 2757 in bd91cb6

	internal::VectorDistributorLocalToGlobal<Number, VectorizedArrayType>
	writer;

for DG/inplace. What do you think? If you agree, I can make the modification.

Alternatively, I was thinking about keeping the internal vectors always clean, i.e., clearing after the usage them, similarly as we do with the ghost values. But I guess in total, we would do more work, since we sometimes simply copy the content of vectors, which does not require zeroing.

[kronbichler]

You mean to use VectorSetter for the case !inplace, right? Because we do prolongate_add, so we need to do the other function.

Regarding clearing the vectors, I would prefer to minimize the work, which means clearing the vectors before use, rather than after. Another reason is that clearing before use is more stable if we start using the vectors in different functions and forget resetting them. The main goal should be to avoid using the temporary vectors and directly work with the dst vector of prolongate_add. We can't do that for all cases, though, so we need the other path.

[kronbichler]

I think the setter would be a good approach.

[peterrum]

!inplace, right?

Right. Let me know if I should implement that. After the recent refactoring, the information is not directly available anymore.

[kronbichler]

I suggest we get this PR in to fix the error, and then look into the possible optimizations later.

[peterrum]

@kronbichler I have fixed the failing test. Now CI should pass and we can merge.

[kronbichler]

Thank you!

[kronbichler]

I am confused why the PR is still shown as open, while https://github.com/dealii/dealii/commits/master shows that it has been merged. Any ideas? I guess we can simply close this one?