#2131: Boundary thorn -------------------------------------------------------+-------------------- Reporter: miguel.zilhao.nogueira@… | Owner: Type: defect | Status: new Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: development version Keywords: Boundary | -------------------------------------------------------+-------------------- i was trying to do a (unigrid) run with "scalar" boundary conditions on some variables but with uneven boundary width, ie width of 2 grid points on the x and y axis and 0 on the z axis. in the documentation of the Boundary thorn it states that one should create a table passing this information in an array called "BOUNDARY_WIDTH":
"The table handle identifies a table which holds extra arguments for the particular boundary condition that is requested. For example, if a negative value is passed for the boundary width, then the boundary condition will look in this table for a 2d-element integer array, which holds the width of each face of the boundary (for a d dimensional grid variable). (The first element of the array holds the width of the ‘-x’ face, the second the ‘+x’ face, the third the ‘-y’ face, etc.)"
so i've accordingly created the following table {{{ call Util_TableCreateFromString(param_table_handle, "BOUNDARY_WIDTH = { 2 2 2 2 0 0 }") {{{ and then registered the variables with {{{ ierr = Boundary_SelectGroupForBC(cctkGH, CCTK_ALL_FACES, -one, & param_table_handle, "ScalarBase::phi", "scalar") }}} however, i was getting errors like the following: {{{ Boundary/src/Check.c:130: BndSanityCheckWidths: Assertion `dim < (int)sizeof(dims)' failed. }}} inspecting that file, this is only triggered if {{{(boundary_widths[i] > 100 || boundary_widths[i] < 0)}}} which meant that my BOUNDARY_WIDTH array was likely not being parsed correctly. digging a little bit deeper, i've found the following in ScalarBoundary.c:138 (and analogous for the other files under CactusBase/Boundary/src): {{{ /* Determine boundary width on all faces */ /* allocate memory for buffer */ gdim = CCTK_GroupDimI(gi); if (gdim > max_gdim) { width_alldirs = (CCTK_INT *)realloc(width_alldirs, 2 * gdim * sizeof(CCTK_INT)); max_gdim = gdim; }
/* fill it with values, either from table or the boundary_width parameter */ if (widths[i] < 0) { err = Util_TableGetIntArray(tables[i], gdim, width_alldirs, "BOUNDARY_WIDTH"); }}} it seems to me that this last line should be instead {{{ err = Util_TableGetIntArray(tables[i], 2 * gdim, width_alldirs, "BOUNDARY_WIDTH"); }}} for consistency with the rest of the file and with the documentation, right? indeed, with this change the errors disappeared. i've attached a simple patch that applies this on this file, but i guess an equivalent change would be needed also for the rest of the Boundary files...
if this patch is correct, could this be ported to the current release? and if it's not correct, is there anything i'm missing, in order to register the boundary conditions?
#2131: Boundary thorn --------------------------------------------------------+------------------- Reporter: miguel.zilhao.nogueira@… | Owner: Type: defect | Status: new Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: Boundary --------------------------------------------------------+-------------------
Comment (by eschnett):
Yes, this looks like an error in the code.
We will also need to have a look at the other files in the same directory, as they might contain the same error.
#2131: Boundary thorn --------------------------------------------------------+------------------- Reporter: miguel.zilhao.nogueira@… | Owner: Type: defect | Status: new Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: Boundary --------------------------------------------------------+------------------- Description changed by rhaas:
Old description:
i was trying to do a (unigrid) run with "scalar" boundary conditions on some variables but with uneven boundary width, ie width of 2 grid points on the x and y axis and 0 on the z axis. in the documentation of the Boundary thorn it states that one should create a table passing this information in an array called "BOUNDARY_WIDTH":
"The table handle identifies a table which holds extra arguments for the particular boundary condition that is requested. For example, if a negative value is passed for the boundary width, then the boundary condition will look in this table for a 2d-element integer array, which holds the width of each face of the boundary (for a d dimensional grid variable). (The first element of the array holds the width of the ‘-x’ face, the second the ‘+x’ face, the third the ‘-y’ face, etc.)"
so i've accordingly created the following table {{{ call Util_TableCreateFromString(param_table_handle, "BOUNDARY_WIDTH = { 2 2 2 2 0 0 }") {{{ and then registered the variables with {{{ ierr = Boundary_SelectGroupForBC(cctkGH, CCTK_ALL_FACES, -one, & param_table_handle, "ScalarBase::phi", "scalar") }}} however, i was getting errors like the following: {{{ Boundary/src/Check.c:130: BndSanityCheckWidths: Assertion `dim < (int)sizeof(dims)' failed. }}} inspecting that file, this is only triggered if {{{(boundary_widths[i] > 100 || boundary_widths[i] < 0)}}} which meant that my BOUNDARY_WIDTH array was likely not being parsed correctly. digging a little bit deeper, i've found the following in ScalarBoundary.c:138 (and analogous for the other files under CactusBase/Boundary/src): {{{ /* Determine boundary width on all faces */ /* allocate memory for buffer */ gdim = CCTK_GroupDimI(gi); if (gdim > max_gdim) { width_alldirs = (CCTK_INT *)realloc(width_alldirs, 2 * gdim * sizeof(CCTK_INT)); max_gdim = gdim; }
/* fill it with values, either from table or the boundary_width parameter */ if (widths[i] < 0) { err = Util_TableGetIntArray(tables[i], gdim, width_alldirs, "BOUNDARY_WIDTH");}}} it seems to me that this last line should be instead {{{ err = Util_TableGetIntArray(tables[i], 2 * gdim, width_alldirs, "BOUNDARY_WIDTH"); }}} for consistency with the rest of the file and with the documentation, right? indeed, with this change the errors disappeared. i've attached a simple patch that applies this on this file, but i guess an equivalent change would be needed also for the rest of the Boundary files...
if this patch is correct, could this be ported to the current release? and if it's not correct, is there anything i'm missing, in order to register the boundary conditions?
New description:
i was trying to do a (unigrid) run with "scalar" boundary conditions on some variables but with uneven boundary width, ie width of 2 grid points on the x and y axis and 0 on the z axis. in the documentation of the Boundary thorn it states that one should create a table passing this information in an array called "BOUNDARY_WIDTH":
"The table handle identifies a table which holds extra arguments for the particular boundary condition that is requested. For example, if a negative value is passed for the boundary width, then the boundary condition will look in this table for a 2d-element integer array, which holds the width of each face of the boundary (for a d dimensional grid variable). (The first element of the array holds the width of the ‘-x’ face, the second the ‘+x’ face, the third the ‘-y’ face, etc.)"
so i've accordingly created the following table {{{ call Util_TableCreateFromString(param_table_handle, "BOUNDARY_WIDTH = { 2 2 2 2 0 0 }") }}} and then registered the variables with {{{ ierr = Boundary_SelectGroupForBC(cctkGH, CCTK_ALL_FACES, -one, & param_table_handle, "ScalarBase::phi", "scalar") }}} however, i was getting errors like the following: {{{ Boundary/src/Check.c:130: BndSanityCheckWidths: Assertion `dim < (int)sizeof(dims)' failed. }}} inspecting that file, this is only triggered if {{{(boundary_widths[i] > 100 || boundary_widths[i] < 0)}}} which meant that my BOUNDARY_WIDTH array was likely not being parsed correctly. digging a little bit deeper, i've found the following in ScalarBoundary.c:138 (and analogous for the other files under CactusBase/Boundary/src): {{{ /* Determine boundary width on all faces */ /* allocate memory for buffer */ gdim = CCTK_GroupDimI(gi); if (gdim > max_gdim) { width_alldirs = (CCTK_INT *)realloc(width_alldirs, 2 * gdim * sizeof(CCTK_INT)); max_gdim = gdim; }
/* fill it with values, either from table or the boundary_width parameter */ if (widths[i] < 0) { err = Util_TableGetIntArray(tables[i], gdim, width_alldirs, "BOUNDARY_WIDTH"); }}} it seems to me that this last line should be instead {{{ err = Util_TableGetIntArray(tables[i], 2 * gdim, width_alldirs, "BOUNDARY_WIDTH"); }}} for consistency with the rest of the file and with the documentation, right? indeed, with this change the errors disappeared. i've attached a simple patch that applies this on this file, but i guess an equivalent change would be needed also for the rest of the Boundary files...
if this patch is correct, could this be ported to the current release? and if it's not correct, is there anything i'm missing, in order to register the boundary conditions?
--
#2131: Boundary thorn --------------------------------------------------------+------------------- Reporter: miguel.zilhao.nogueira@… | Owner: Type: defect | Status: confirmed Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: Boundary --------------------------------------------------------+------------------- Changes (by rhaas):
* status: new => confirmed
#2131: Boundary thorn ---------------------------------------+--------------------------------- Reporter: miguel.zilhao.nogueira@… | Owner: (none) Type: defect | Status: confirmed Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: Boundary ---------------------------------------+---------------------------------
Comment (by Roland Haas):
Miguel, Erik: is there any progress on this?
#2131: Boundary thorn ---------------------------------------+--------------------------------- Reporter: miguel.zilhao.nogueira@… | Owner: (none) Type: defect | Status: confirmed Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: Boundary ---------------------------------------+---------------------------------
Comment (by Roland Haas):
Miguel, Erik: since you have already proposed a solution, would you like to propose a patch so that it could potentially be included in the next release?
#2131: Boundary thorn ---------------------------------------+--------------------------------- Reporter: miguel.zilhao.nogueira@… | Owner: (none) Type: defect | Status: confirmed Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: Boundary ---------------------------------------+--------------------------------- Changes (by miguel.zilhao.nogueira@…):
* Attachment "boundary.patch" added.
#2131: Boundary thorn ---------------------------------------+--------------------------------- Reporter: miguel.zilhao.nogueira@… | Owner: (none) Type: defect | Status: confirmed Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: Boundary ---------------------------------------+---------------------------------
Comment (by miguel.zilhao.nogueira@…):
There was already one patch proposed, but it only fixed one file. I've just added another patch, fixing all the files affected.
#2131: Boundary thorn ---------------------------------------+--------------------------------- Reporter: miguel.zilhao.nogueira@… | Owner: (none) Type: defect | Status: review Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: Boundary ---------------------------------------+--------------------------------- Changes (by Roland Haas):
* status: confirmed => review
#2131: Boundary thorn ---------------------------------------+--------------------------------- Reporter: miguel.zilhao.nogueira@… | Owner: (none) Type: defect | Status: reviewed_ok Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: Boundary ---------------------------------------+--------------------------------- Changes (by Roland Haas):
* status: review => reviewed_ok
Comment:
The patch looks fine to me.
Please apply.
Or let me know if you do not have commit rights and I will apply the patch with you as the author.
#2131: Boundary thorn ---------------------------------------+--------------------------------- Reporter: miguel.zilhao.nogueira@… | Owner: (none) Type: defect | Status: reviewed_ok Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: Boundary ---------------------------------------+---------------------------------
Comment (by miguel.zilhao.nogueira@…):
I do not have commit rights... Could you take care of it?
#2131: Boundary thorn ---------------------------------------+--------------------------------- Reporter: miguel.zilhao.nogueira@… | Owner: (none) Type: defect | Status: closed Priority: major | Milestone: Component: EinsteinToolkit thorn | Version: development version Resolution: fixed | Keywords: Boundary ---------------------------------------+--------------------------------- Changes (by Roland Haas):
* status: reviewed_ok => closed * resolution: => fixed
Comment:
Applied as git hash 08315df "Boundary: correct size of width_alldirs data array" of cactusbase.
Thank you.
I did not mention this, but one could have also created a pull request on bitbucket which anyone of the developers could then merge in (ideally using the fast-forward merge method to avoid having one additional merge commit for every pull requested commit).
trac@lists.einsteintoolkit.org