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

Return composite sizes from geometricField #169

Open
greole opened this issue Oct 25, 2024 · 2 comments
Open

Return composite sizes from geometricField #169

greole opened this issue Oct 25, 2024 · 2 comments
Labels
good first issue Good for newcomers

Comments

@greole
Copy link
Contributor

greole commented Oct 25, 2024

Currently, when calling size() for GeometricFields the internalField size is returned. While this is often what user intends it is, however, ambiguous. One better solution would be to return:

a. a tuple<size_t, size_t> with the internalField and boudaryField size
b. a struct GeometricFieldSize which holds both values.

in both cases one has to explicitely select which size is desired.

@greole greole added the good first issue Good for newcomers label Oct 25, 2024
@bevanwsjones bevanwsjones self-assigned this Nov 5, 2024
@bevanwsjones
Copy link
Collaborator

I had a look into this, as I was hoping to do a bit of warm-up on it 😉. I am not sure doing this makes sense, because the boundaryFields is not a Field in the same way that the InternalField is a Field. I would suggest we actually change the BoundaryFields name to BoundaryFaceConditions or FaceBoundaryConditions or something similar. It does not really represent a field per se; it's more of a boundary condition container that may contain values of size faces.

@bevanwsjones bevanwsjones removed their assignment Nov 5, 2024
@greole
Copy link
Contributor Author

greole commented Nov 5, 2024

Yes I strongly agree with renaming boundaryFields,

  1. I think we have too many classes with the name field, which aren't fields. To me a field is a container which supports operations +, *
  2. How about BoundaryValueContainer ? Because as you say it is a container that holds all required values to compute the boundary conditions. That would be also my argument against BoundaryFaceConditions because a. it doensn't know anything about the actual condition and b. maybe we Face could be misleading.

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

No branches or pull requests

2 participants