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

Select quadrature data domain #877

Closed
ericneiva opened this issue Feb 21, 2023 · 2 comments
Closed

Select quadrature data domain #877

ericneiva opened this issue Feb 21, 2023 · 2 comments

Comments

@ericneiva
Copy link
Member

ericneiva commented Feb 21, 2023

Hi, @amartinhuertas,

Some time ago, you added in a6cc908 a trait template parameter to CellQuadrature. It lets the user select whether to integrate in reference or physical domain.

Conversely, the default domain for the quad data is reference and it cannot be changed at the user level 👇

function CellQuadrature(trian::Triangulation,
cell_quad::AbstractVector{<:Quadrature},ids::DomainStyle)
ctype_to_quad, cell_to_ctype = compress_cell_data(cell_quad)
ctype_to_point = map(get_coordinates,ctype_to_quad)
ctype_to_weight = map(get_weights,ctype_to_quad)
cell_point = expand_cell_data(ctype_to_point,cell_to_ctype)
cell_weight = expand_cell_data(ctype_to_weight,cell_to_ctype)
CellQuadrature(cell_quad,cell_point,cell_weight,trian,ReferenceDomain(),ids)
end

The method above is the only one that calls the constructor (see line 56). As you can see it sets the data domain style to ReferenceDomain().

I am interfacing Gridap to a library that computes quad points and weights directly in the physical domain. I can make it work, but I need to replicate the above method to set the data domain to physical.

Have you thought about letting the user have power over both data and integration domain styles? I thought I could change the API of CellQuadrature such that both domain styles are keyword arguments. What do you think?

@amartinhuertas
Copy link
Member

Hi @ericneiva,

thanks for your issue, and sorry for the very late reply (the last months have been quite hectic for me, but anyway ...)

Let me answer to your comments one by one.

The method above is the only one that calls the constructor (see line 56). As you can see it sets the data domain style to ReferenceDomain().

Let me note that apart from a6cc908, there is another related commit, namely, 5167a37. In this later commit, I recovered the signature of an older outer constructor (for backward compatibility). This outer constructor also calls the inner constructor, and lets you specify DataDomainStyle. In any case, this outer constructor does not provide extra expressivity as it is possible to call the inner constructor passing all the fields of the structure. (i.e., the default inner constructor).

I am interfacing Gridap to a library that computes quad points and weights directly in the physical domain. I can make it work, but I need to replicate the above method to set the data domain to physical.

If am not missing anything, I don't see why there is a need. You can still leverage the default inner constructor of CellQuadrature from Measure, e.g.:

Measure(cell_quad, cell_point, cell_weight, trian, PhysicalDomain(), PhysicalDomain())

Have you thought about letting the user have power over both data and integration domain styles? I thought I could change the API of CellQuadrature such that both domain styles are keyword arguments. What do you think?

The user has already this power. See my point above. In any case, I see that this is a quite inconvenient low-level API for the user as you have to provide all the details of the CellQuadrature from Measure, while there is part of the work that Gridap can do for you . Was this your main concern? As far I understand, you would like something as, e.g.: (I am aware you have proposed something different in the PR):

Measure(trian, degree, PhysicalDomain(), PhysicalDomain())

Am I correct?

Now let me comment on PR #885 there. (btw thanks for the PR).

@ericneiva
Copy link
Member Author

Hi, @amartinhuertas, thank you so much for your reply.

In any case, I see that this is a quite inconvenient low-level API for the user as you have to provide all the details of the CellQuadrature from Measure, while there is part of the work that Gridap can do for you . Was this your main concern?

Spot on.

sorry for the very late reply

It's alright 😄 I completely understand.

I think we can finish the discussion at PR #885.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants