Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MGTwoLevelTransfer: enable FE_Nothing #12891

Merged
merged 1 commit into from Oct 28, 2021
Merged

Conversation

peterrum
Copy link
Member

No description provided.

Copy link
Member

@kronbichler kronbichler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some random comments, looks like a useful addition! 👍

Comment on lines 1640 to 1641
for (unsigned int i = 0; i < fe_collection.size(); ++i)
result &= fu(fe_collection[i]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be easier or at least more readable with

Suggested change
for (unsigned int i = 0; i < fe_collection.size(); ++i)
result &= fu(fe_collection[i]);
for (unsigned int i = 0; i < fe_collection.size(); ++i)
if (!fu(fe_collection[i]))
return false;

and return true otherwise?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment on lines 1777 to 1779
Assert(reference_cell ==
dof_handler_coarse.get_fe(fe_index_pair.first.second)
dof_handler_coarse.get_fe(fe_index_pair.first.first)
.reference_cell(),
ExcNotImplemented());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this was a bug before, but we never actually triggered the alternative element?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It was "just" an assert ;)

source/base/data_out_base.cc Outdated Show resolved Hide resolved
@peterrum peterrum changed the title [WIP] MGTwoLevelTransfer: enable FE_Nothing MGTwoLevelTransfer: enable FE_Nothing Oct 27, 2021
@peterrum
Copy link
Member Author

@kronbichler I have updated the PR. Ready from my side.

@peterrum
Copy link
Member Author

/rebuild

Copy link
Member

@kronbichler kronbichler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, apart from some comments regarding the test.



/**
* TODO
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you describe what the test is doing?

Comment on lines +53 to +70
template <int dim>
class AnalyticalSolution : public Function<dim>
{
public:
AnalyticalSolution()
{}

virtual double
value(const Point<dim> &p, const unsigned int component = 0) const override
{
(void)component;

return p[0];
}

private:
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using

Monomial(const Tensor<1, dim, Number> &exponents,
const unsigned int n_components = 1);
with exponents = {1, 0, 0}?

@peterrum peterrum merged commit 7f8de9e into dealii:master Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants