-
Notifications
You must be signed in to change notification settings - Fork 33
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
patchkernel: introduce LineKernel and PointKernel classes #29
Conversation
57bba6f
to
a3b774e
Compare
a3b774e
to
ccf75e9
Compare
ccf75e9
to
a012d85
Compare
@roccoarpa @edoardolombardi Do you have an estimate on when we can do the merge? |
@andrea-iob @edoardolombardi sorry for that, we will have meetings in the next few days to update priorities on mimmo. I will try to insert this pull check (coming with major mimmo rework) on them as urgent task, but I'm afraid is not the top priority in the short period. Just to answer your question, I will say 2/3 months from now as reasonable deadline, maybe earlier if some time slots get free in the meantime. I cannot be more specific right now. |
a012d85
to
7797473
Compare
7797473
to
20a6e1a
Compare
20a6e1a
to
e13ae33
Compare
e13ae33
to
1bf02ed
Compare
1bf02ed
to
d807b9c
Compare
d807b9c
to
cb072bd
Compare
This pull request is here from a very long time. If it's not possible to merge the whole pull request, can we merge at least the commits that introduce the LineUnstructured and PointClound modules? It would be good to merge also the commit remove the space dimension parameter from the SurfaceKernel module (this changes the signature of SurfaceKernel constructor). |
@andrea-iob as we discussed, if the patches are strict to the related codimension and they don't allow to put mixed higher codimensional element in the same patch (e.g. a 3D surface patch could have 3D surface elements, 2D line elements and 1D vertex elements), personally, I cannot approve the pull request. For the purposes of our applications , mixed-codimensional elements have to be supported at least for one type of patch. |
This is the first time I hear that the reason why this pull request has not been approved it's because it limits the type of elements that can be added to a patch. This limitation is only introduced in the last commit. I've no problems removing that commit (cb072bd). We can also ditch that commits that change the argument passed to extractEdgeNetwork and extractEnvelope ( |
For me it's ok, if the compatibility with mixed-codimension is preserved we can modify as you suggested to go forward with the pull. |
…class Surfacekernel objects are now defined as objects with a codimension, in the corresponding VolumeKernel space, equal to one.
cb072bd
to
74aff67
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried only the compatibility of codes using SurfUnstructured as before, i.e. with mixed elements. No LineKernel & PointKernel embedded in codes and tested outside bitpit.
The class LineKernel allows to define patches that have a co-dimension equal to two with respect to space of a patch (for a three-dimensional patch the LineKernel may be used to define segments, for a two-dimensional patch the LineKernel may be used to define points).
The class PointKernel allows to define patches that have a co-dimension equal to three with respect to space of a patch (for a three-dimensional patch the LineKernel may be used to define points, whereas it doesn't make sens to define a PointKernel for a two-dimensional patch).