librelist archives

« back to archive

ghost_to_index removed

ghost_to_index removed

From:
Lucas Wilcox
Date:
2014-10-23 @ 17:58
Hi,

Is there a reason ghost_to_index was removed from p4est_mesh_t?  I was
using it in when connecting to p4est.

Thanks,
Lucas

Re: [p4est] ghost_to_index removed

From:
Carsten Burstedde
Date:
2014-10-23 @ 18:17
Hi Lucas,

> Is there a reason ghost_to_index was removed from p4est_mesh_t?  I was
> using it in when connecting to p4est.

I redesigned it a bit a while ago -- with ghost_to_proc being there,
what functionality would you request from ghost_to_index?  With the
number of the ghost you can index into p4est_ghost_t yourself if you
need anything else.  Please let me know if I'm missing something.

Best,

Carsten

Re: [p4est] ghost_to_index removed

From:
Lucas Wilcox
Date:
2014-10-23 @ 19:30
On Thu, Oct 23, 2014 at 08:17:46PM +0200, Carsten Burstedde wrote:
> Hi Lucas,
> 
> > Is there a reason ghost_to_index was removed from p4est_mesh_t?  I was
> > using it in when connecting to p4est.
> 
> I redesigned it a bit a while ago -- with ghost_to_proc being there,
> what functionality would you request from ghost_to_index?  With the
> number of the ghost you can index into p4est_ghost_t yourself if you
> need anything else.  Please let me know if I'm missing something.

Sure, I can index into the ghosts array and get the owner's local id
from the piggy data attached to the quadrant.  I just found it
convenient to have ghost_to_index.

I guess, my more fundamental question is, what is the role of
p4est_mesh_t?  What data should/will it include in the future?
As all of the information can be generated from forest and ghost
layer with various levels of effort.

I don't want to keep depending on p4est_mesh_t if it is not going to be
around in the future.

Best,
Lucas

Re: [p4est] ghost_to_index removed

From:
Carsten Burstedde
Date:
2014-10-23 @ 20:01
Hi Lucas,

> > > Is there a reason ghost_to_index was removed from p4est_mesh_t?  I was
> > > using it in when connecting to p4est.
> > 
> > I redesigned it a bit a while ago -- with ghost_to_proc being there,
> > what functionality would you request from ghost_to_index?  With the
> > number of the ghost you can index into p4est_ghost_t yourself if you
> > need anything else.  Please let me know if I'm missing something.
> 
> Sure, I can index into the ghosts array and get the owner's local id
> from the piggy data attached to the quadrant.  I just found it
> convenient to have ghost_to_index.

would a function/macro still do this conveniently enough?

> I guess, my more fundamental question is, what is the role of
> p4est_mesh_t?  What data should/will it include in the future?
> As all of the information can be generated from forest and ghost
> layer with various levels of effort.

It's supposed to provide O(1) lookups that are otherwise only available
from inside the iterator or by O(log) searches.  I'm trying to keep it
non-redundant and memory-efficient as much as possible.

> I don't want to keep depending on p4est_mesh_t if it is not going to be
> around in the future.

Oh, it will be around.  It took me a while to encode the corners; the
edge handling in 3D is still missing.  Do you have further suggestions?

Best,

Carsten

Re: [p4est] ghost_to_index removed

From:
Tobin Isaac
Date:
2014-10-23 @ 20:08
I am in favor of shifting the user interface away from direct struct 
access wherever possible.

  Toby

On October 23, 2014 3:01:57 PM CDT, Carsten Burstedde 
<burstedde@ins.uni-bonn.de> wrote:
>Hi Lucas,
>
>> > > Is there a reason ghost_to_index was removed from p4est_mesh_t? 
>I was
>> > > using it in when connecting to p4est.
>> > 
>> > I redesigned it a bit a while ago -- with ghost_to_proc being
>there,
>> > what functionality would you request from ghost_to_index?  With the
>> > number of the ghost you can index into p4est_ghost_t yourself if
>you
>> > need anything else.  Please let me know if I'm missing something.
>> 
>> Sure, I can index into the ghosts array and get the owner's local id
>> from the piggy data attached to the quadrant.  I just found it
>> convenient to have ghost_to_index.
>
>would a function/macro still do this conveniently enough?
>
>> I guess, my more fundamental question is, what is the role of
>> p4est_mesh_t?  What data should/will it include in the future?
>> As all of the information can be generated from forest and ghost
>> layer with various levels of effort.
>
>It's supposed to provide O(1) lookups that are otherwise only available
>from inside the iterator or by O(log) searches.  I'm trying to keep it
>non-redundant and memory-efficient as much as possible.
>
>> I don't want to keep depending on p4est_mesh_t if it is not going to
>be
>> around in the future.
>
>Oh, it will be around.  It took me a while to encode the corners; the
>edge handling in 3D is still missing.  Do you have further suggestions?
>
>Best,
>
>Carsten

p4est read access interface

From:
Carsten Burstedde
Date:
2014-10-24 @ 10:58
On Thu, Oct 23, 2014 at 03:08:09PM -0500, Tobin Isaac wrote:
> I am in favor of shifting the user interface away from direct struct 
access wherever possible.

The low-level access has been part of the success of p4est from the beginning.
Library developers are likely to write their own glue code, where an additional
interface layer on our side would cause overhead.  On the other hand, direct
access is error-prone and hard to understand for the casual user, and may
offend people based on their coding philosophy.  What about a
best-of-both-worlds approach:

Keep p4est working internally with direct struct access, this applies to all
current files.  Add a file p4est_access or similar that provides function-based
interfaces for read access and iterators for all major data structures and is
exceptionally well documented.

Comments?

Carsten

p4est read access interface

From:
Carsten Burstedde
Date:
2014-10-24 @ 10:26
On Thu, Oct 23, 2014 at 03:08:09PM -0500, Tobin Isaac wrote:
> I am in favor of shifting the user interface away from direct struct 
access wherever possible.

The low-level access has been part of the success of p4est from the beginning.
Library developers are likely to write their own glue code, where an additional
interface layer on our side would cause overhead.  On the other hand, direct
access is error-prone and hard to understand for the casual user, and may
offend people based on their coding philosophy.  What about a
best-of-both-worlds approach:

Keep p4est working internally with direct struct access, this applies to all
current files.  Add a file p4est_access or similar that provides function-based
interfaces for read access and iterators for all major data structures and is
exceptionally well documented.

Comments?

Carsten

Re: [p4est] p4est read access interface

From:
Tobin Isaac
Date:
2014-10-24 @ 11:54
On Fri, Oct 24, 2014 at 12:26:49PM +0200, Carsten Burstedde wrote:
> On Thu, Oct 23, 2014 at 03:08:09PM -0500, Tobin Isaac wrote:
> > I am in favor of shifting the user interface away from direct struct 
access wherever possible.
> 
> The low-level access has been part of the success of p4est from the beginning.
> Library developers are likely to write their own glue code, where an additional
> interface layer on our side would cause overhead.  On the other hand, direct
> access is error-prone and hard to understand for the casual user, and may
> offend people based on their coding philosophy.  What about a
> best-of-both-worlds approach:
> 
> Keep p4est working internally with direct struct access, this applies to all
> current files.  Add a file p4est_access or similar that provides function-based
> interfaces for read access and iterators for all major data structures and is
> exceptionally well documented.
> 
> Comments?

The coding philosophy part doesn't bother me, it's the error-prone
part.  A p4est_access would be good if we committed to it, but
xkcd.com/927/ ...

The main thing that I'm worried about is that, as struct members
become part of the interface, it becomes more difficult to try out
better algorithms/implementations in future releases, because we have
to worry about breaking users' code.  Obviously with p4est_quadrant_t,
this isn't a concern, but as the last thread with Lucas shows, it is a
problem for the more peripheral structs that proliferate over the life
of the project.

  Toby

> 
> Carsten

Re: [p4est] p4est read access interface

From:
Lucas Wilcox
Date:
2014-10-24 @ 19:04
On Fri, Oct 24, 2014 at 06:54:55AM -0500, Tobin Isaac wrote:
> The main thing that I'm worried about is that, as struct members
> become part of the interface, it becomes more difficult to try out
> better algorithms/implementations in future releases, because we have
> to worry about breaking users' code.  Obviously with p4est_quadrant_t,
> this isn't a concern, but as the last thread with Lucas shows, it is a
> problem for the more peripheral structs that proliferate over the life
> of the project.

I second the no strut members for the ease of wrapping the p4est API for
other languages.  I recently did some Fortran and Julia bindings (not
for p4est) and have come to realize simple APIs are a lot easier to
access from various languages.

Re: [p4est] p4est read access interface

From:
Carsten Burstedde
Date:
2014-10-26 @ 15:34
> The coding philosophy part doesn't bother me, it's the error-prone
> part.  A p4est_access would be good if we committed to it, but
> xkcd.com/927/ ...

Very good point.  I'd propose to make a joint effort to finalize the
struct members for v2.0, and then think about a consistent _access
interface separately.

Carsten

Re: [p4est] ghost_to_index removed

From:
Lucas Wilcox
Date:
2014-10-23 @ 21:27
On Thu, Oct 23, 2014 at 10:01:57PM +0200, Carsten Burstedde wrote:
> would a function/macro still do this conveniently enough?

Well, as it is right now I need to directly access the piggy from the
struct.  So maybe a macro is in order?  What do you think Toby?


> > I guess, my more fundamental question is, what is the role of
> > p4est_mesh_t?  What data should/will it include in the future?
> > As all of the information can be generated from forest and ghost
> > layer with various levels of effort.
> 
> It's supposed to provide O(1) lookups that are otherwise only available
> from inside the iterator or by O(log) searches.  I'm trying to keep it
> non-redundant and memory-efficient as much as possible.

That seems reasonable.  Why did the vertices and quad_to_vertices arrays
get removed in this case?  I know that I can generate them again by
calling p4est_nodes_new().

> > I don't want to keep depending on p4est_mesh_t if it is not going to be
> > around in the future.
> 
> Oh, it will be around.  It took me a while to encode the corners; the
> edge handling in 3D is still missing.  Do you have further suggestions?

Okay good.  I just wanted to double check as I am changing my code now
and don't want to touch it again if/when possible.

Best,
Lucas

Re: [p4est] ghost_to_index removed

From:
Carsten Burstedde
Date:
2014-10-24 @ 10:19
On Thu, Oct 23, 2014 at 02:27:02PM -0700, Lucas Wilcox wrote:
> On Thu, Oct 23, 2014 at 10:01:57PM +0200, Carsten Burstedde wrote:
> > would a function/macro still do this conveniently enough?
> 
> Well, as it is right now I need to directly access the piggy from the
> struct.  So maybe a macro is in order?  What do you think Toby?

Let's start a new thread on this interface issue.  It's an important
point we should talk about for all of p4est.

> > > I guess, my more fundamental question is, what is the role of
> > > p4est_mesh_t?  What data should/will it include in the future?
> > > As all of the information can be generated from forest and ghost
> > > layer with various levels of effort.
> > 
> > It's supposed to provide O(1) lookups that are otherwise only available
> > from inside the iterator or by O(log) searches.  I'm trying to keep it
> > non-redundant and memory-efficient as much as possible.
> 
> That seems reasonable.  Why did the vertices and quad_to_vertices arrays
> get removed in this case?  I know that I can generate them again by
> calling p4est_nodes_new().

Do the mesh->corner type arrays work for you?  They identify nodes by
periodicity so maybe that's not what you want.  So it seems having a
vertex lookup in the geometric sense would have added value, but it
is not cheap memory-wise.

Now, should we isolate a p4est_vertices_new function by ripping
nodes_new?  Or add additional fields to the mesh, filled only if needed
as by another boolean parameter in mesh_new_ext?  Maybe the second
option keeps the interface more concise, having the mesh as a one-stop
configurable data structure.

> > > I don't want to keep depending on p4est_mesh_t if it is not going to be
> > > around in the future.
> > 
> > Oh, it will be around.  It took me a while to encode the corners; the
> > edge handling in 3D is still missing.  Do you have further suggestions?
> 
> Okay good.  I just wanted to double check as I am changing my code now
> and don't want to touch it again if/when possible.

It shouldn't change much, but we may add to it in the future.

Carsten

Re: [p4est] ghost_to_index removed

From:
Lucas Wilcox
Date:
2014-10-24 @ 19:00
On Fri, Oct 24, 2014 at 12:19:36PM +0200, Carsten Burstedde wrote:
> > That seems reasonable.  Why did the vertices and quad_to_vertices arrays
> > get removed in this case?  I know that I can generate them again by
> > calling p4est_nodes_new().
> 
> Do the mesh->corner type arrays work for you?  They identify nodes by
> periodicity so maybe that's not what you want.  So it seems having a
> vertex lookup in the geometric sense would have added value, but it
> is not cheap memory-wise.

Is quad_to_corner supposed to be the same as quad_to_vertices?  I shied
away from them because of the warning in the header:

    CAUTION: tree-boundary corners not yet implemented

> Now, should we isolate a p4est_vertices_new function by ripping
> nodes_new?  Or add additional fields to the mesh, filled only if needed
> as by another boolean parameter in mesh_new_ext?  Maybe the second
> option keeps the interface more concise, having the mesh as a one-stop
> configurable data structure.

That I don't know.  For now I can just use nodes_new.  I just wanted to
give my experience about upgrading a code using p4est.  Granted this was
from a pre 1.0 release.

Re: [p4est] ghost_to_index removed

From:
Carsten Burstedde
Date:
2014-10-26 @ 15:32
> Is quad_to_corner supposed to be the same as quad_to_vertices?  I shied
> away from them because of the warning in the header:

I believe it's not.  Vertices and corners are each interpreted in the
sense described for the connectivity.  If we want the option to populate
the vertices as well, what would you think about another boolean
parameter for mesh_new_ext that would trigger nodes_new (ghost = NULL)?
I must say I would not want to rely on the nodes_t type and rather
cannibalize nodes_new_local for a reasonable set of added mesh members.

>     CAUTION: tree-boundary corners not yet implemented

Check out the wrap branch -- for 2D the functionality is complete.  3D
needs edge logic that I'm not getting to soon myself.  It should not be
too hard to write this though given the code that's there already.

> > Now, should we isolate a p4est_vertices_new function by ripping
> > nodes_new?  Or add additional fields to the mesh, filled only if needed
> > as by another boolean parameter in mesh_new_ext?  Maybe the second
> > option keeps the interface more concise, having the mesh as a one-stop
> > configurable data structure.
> 
> That I don't know.  For now I can just use nodes_new.  I just wanted to
> give my experience about upgrading a code using p4est.  Granted this was
> from a pre 1.0 release.

It seems we all would like to tweak the struct API just a bit more
before we call it 2.0 and stick with it.  Will start another thread
about this.

Carsten