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

FE_Q_iso_Q1: varying subdivisions #12838

Merged

Conversation

peterrum
Copy link
Member

@peterrum peterrum commented Oct 16, 2021

The aim is:

const unsigned int n_subdivisions = 5;
QGaussLobatto<1> q_outer(n_subdivisions);
FE_Q_iso_Q1<dim> fe_q1(q_outer.get_points());

None: I have introduced the class PiecewiseLinearPolynomial. But I guess it would be better to introduce the functionality in PiecewisePolynomial to prevent the need to perform dynamic_casts to the new class. Opinions?

@drwells
Copy link
Member

drwells commented Oct 16, 2021

Why can't we just use PiecewisePolynomial here?

@peterrum
Copy link
Member Author

Why can't we just use PiecewisePolynomial here?

It assumes uniform subdivisions.

@peterrum peterrum force-pushed the FE_Q_iso_Q1_varying_subdivisions branch 2 times, most recently from bf55437 to 71ac540 Compare October 17, 2021 12:16
Comment on lines 246 to 248
1.0 - (x - points[index][0]) /
(points[index + 1][0] -
points[index][0]));
Copy link
Member

Choose a reason for hiding this comment

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

This division can potentially be expensive, as we might call value in inner loops. Do you think it would make sense to store a vector of one_over_lengths to ensure we only do multiplications?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll do that later.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have made the changes. Furthermore, I have added a test.

@peterrum peterrum force-pushed the FE_Q_iso_Q1_varying_subdivisions branch from 71ac540 to dbc4da7 Compare October 19, 2021 12:48
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, but let us wait for one more voice.

@kronbichler
Copy link
Member

/rebuild

Copy link
Member

@Rombur Rombur 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 except for a couple of typos

include/deal.II/base/polynomials_piecewise.h Outdated Show resolved Hide resolved
include/deal.II/base/polynomials_piecewise.h Outdated Show resolved Hide resolved
include/deal.II/base/polynomials_piecewise.h Outdated Show resolved Hide resolved
@peterrum peterrum force-pushed the FE_Q_iso_Q1_varying_subdivisions branch from dbc4da7 to d122a38 Compare October 22, 2021 17:24
@peterrum
Copy link
Member Author

@Rombur Thanks for the review. I have fixed the typos!

@kronbichler kronbichler merged commit 5d7f369 into dealii:master Oct 25, 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

4 participants