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.

Sign up for GitHub

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

Jump to bottom

Tensor product matrix creator #14291

Closed
wants to merge 0 commits into from
Closed

Tensor product matrix creator #14291

wants to merge 0 commits into from

Conversation

peterrum
Copy link
Member

depends on #14288 and #14289

@kronbichler
Copy link
Member

Can you rebase this so I can have a new look?

@peterrum
Copy link
Member Author

Can you rebase this so I can have a new look?

Done!

kronbichler
kronbichler reviewed Sep 22, 2022
View reviewed changes
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.

I think this makes sense, but I do not think this should reside in the lac subfolder. How about moving it to FETools or something like that?

include/deal.II/lac/tensor_product_matrix.h Outdated Show resolved Hide resolved
include/deal.II/lac/tensor_product_matrix.h Outdated Show resolved Hide resolved
include/deal.II/lac/tensor_product_matrix.h Outdated Show resolved Hide resolved
include/deal.II/lac/tensor_product_matrix.h Outdated Show resolved Hide resolved
include/deal.II/lac/tensor_product_matrix.h Outdated Show resolved Hide resolved
include/deal.II/lac/tensor_product_matrix.h Outdated
std::array<FullMatrix<Number>, dim>>
create_laplace_tensor_product_matrix(
const typename Triangulation<dim>::cell_iterator &cell,
const std::set<types::boundary_id> & dbcs,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const std::set<types::boundary_id> & dbcs,
const std::set<types::boundary_id> & dirichlet_boundaries,

and similar for the Neumann case; what would you do in the Robin boundary case?

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 need to think about this.

include/deal.II/lac/tensor_product_matrix.h Outdated Show resolved Hide resolved
include/deal.II/lac/tensor_product_matrix.h Outdated Show resolved Hide resolved
include/deal.II/lac/tensor_product_matrix.h Outdated Show resolved Hide resolved
include/deal.II/lac/tensor_product_matrix.h Outdated Show resolved Hide resolved
@peterrum
Copy link
Member Author

peterrum commented Sep 22, 2022 •
edited

I think this makes sense, but I do not think this should reside in the lac subfolder.

I have moved it to the numerics folder.

kronbichler
kronbichler approved these changes Sep 24, 2022
View reviewed changes
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.

This looks good in my opinion. I think one question I would have for the future is whether there is a way to avoid re-computing the same reference mass and stiffness matrices, as we might call this function many times with just the cell_extent and boundary_ids differing, but the same underlying 1D matrices.

include/deal.II/numerics/tensor_product_matrix_creator.h Outdated Show resolved Hide resolved
@peterrum
Copy link
Member Author

/rebuild

@peterrum peterrum force-pushed the TensorProductMatrixCreator branch 3 times, most recently from 2c8ee14 to 71ca9c4 Compare September 25, 2022 15:21
kronbichler
kronbichler approved these changes Sep 27, 2022
View reviewed changes
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.

Do you plan tests for this? I think we should move forward here, so we can do it elsewhere. But you should rebase to get the CI going, not sure what happened.

@peterrum peterrum closed this Sep 27, 2022
@peterrum peterrum mentioned this pull request Sep 27, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kronbichler kronbichler kronbichler approved these changes

Assignees
No one assigned
Labels
Linear Algebra ready to test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@peterrum @kronbichler