fix a bug found by Yosef Zlochower:

   In a multiprocessor run, AHFinderDirect finds different horizons
   in parallel on different processors, and doesn't broadcast horizon
   shapes to all processors unless it thinks they'll be needed.  However,
   the "the information will be needed" calculation doesn't take into
   account the possibility of the  HorizonRadiusInDirection()  aliased
   function being called on another processor, and in this case this
   function returns a wrong result if AHFinderDirect hasn't broadcast
   the horizon shapes.

* introduce a new Boolean parameter   always_broadcast_horizon_shape
  (defaults to false)
* document in interface.ccl, param.ccl, and doc/documentation.tex,
  that this parameter should be set to  true  if you expect
  HorizonRadiusInDirection() to be used
* inclusive-or this parameter into the "should we broadcast the horizon shape?"
  calculation in  src/driver/find_horizons.cc


git-svn-id: http://svn.einsteintoolkit.org/cactus/EinsteinAnalysis/AHFinderDirect/trunk@1419 f88db872-0e4f-0410-b76b-b9085cfa78c5

1 files changed, 46 insertions, 22 deletions

diff --git a/param.ccl b/param.ccl
index 14474a4..57b0b94 100644
--- a/param.ccl
+++ b/param.ccl
@@ -59,10 +59,10 @@ int find_every "how often should we try to find apparent horizons?"
 #
 Boolean run_at_CCTK_ANALYSIS "should we run in CCTK_ANALYSIS schedule bin?"
 {
-} "true"
+} true
 Boolean run_at_CCTK_POSTSTEP "should we run in CCTK_POSTSTEP schedule bin?"
 {
-} "false"
+} false
 
 #
 # unless you're trying to debug AHFinderDirect, you should leave
@@ -135,6 +135,30 @@ int which_surface_to_store_info[101] \
 } -1
 
 #
+# When finding multiple horizons in a multiprocessor run, this thorn
+# will find different horizons in parallel on different processors.
+# As a performance optimization, this thorn normally only broadcasts
+# each horizon's shape from the processor that finds it, to all processors,
+# if it thinks that information will be needed, i.e. if either or both
+# of the following conditions hold:
+# * we're setting an excision mask
+# * we're storing horizon information in the SphericalSurface variables
+#   for at least one horizon
+#
+# If another thorn calls the  HorizonRadiusInDirection()  aliased function
+# for a horizon which was found on some other processor, this broadcast is
+# also needed to get the right answer (in a multiprocessor run), but this
+# thorn doesn't know this.  The following parameter should be used in this
+# case to force the broadcast to be made.
+#
+Boolean always_broadcast_horizon_shape \
+  "should we always broadcast each horizon shape from the processor \
+   that finds it, to all processors, even if there's no apparent need \
+   for this information on other processors?"
+{
+} false
+
+#
 # This parameter controls how verbose this thorn is in printing
 # informational (non-error) messages describing what it's doing.
 #
@@ -165,7 +189,7 @@ keyword verbose_level \
 Boolean print_timing_stats \
   "should we print timing stats for the whole apparent-horizon-finding process?"
 {
-} "false"
+} false
 
 # By default, we find apparent horizons by solving the equation
 #  Theta(h) = 0
@@ -331,7 +355,7 @@ real initial_guess__coord_ellipsoid__z_radius[101] "z radius of ellipsoid"
 Boolean output_BH_diagnostics \
   "should we output BH diagnostics to a data file for each AH found?"
 {
-} "true"
+} true
 
 #
 # These file names are actually just "base" file names, with the full
@@ -404,7 +428,7 @@ Boolean output_ghost_zones_for_h \
   "should we include the patch system (angular) interpatch ghost zones \
    in h data files?"
 {
-} "false"
+} false
 
 # n.b. making this "gnuplot" probably isn't a good idea,
 #      since it could create confusion with gnuplot command files
@@ -474,7 +498,7 @@ Boolean output_OpenDX_control_files \
   "should we output OpenDX control files to allow reading of AHFinderDirect \
    'ASCII (gnuplot)' format data files?"
 {
-} "true"
+} true
 string OpenDX_control_file_name_extension \
   "file name extension for OpenDX control files"
 {
@@ -491,7 +515,7 @@ string OpenDX_control_file_name_extension \
 Boolean output_initial_guess \
   "should we output the initial guess back to the h data file?"
 {
-} "false"
+} false
 
 # for debugging convergence failures, we can optionally output
 # h, Theta, and delta_h at each Newton iteration
@@ -499,7 +523,7 @@ Boolean output_initial_guess \
 Boolean debugging_output_at_each_Newton_iteration \
   "should we output {h, Theta, delta_h} at each Newton iteration?"
 {
-} "false"
+} false
 
 string Jacobian_base_file_name "base file name for Jacobian output file(s)"
 {
@@ -516,12 +540,12 @@ string Jacobian_base_file_name "base file name for Jacobian output file(s)"
 Boolean set_mask_for_all_horizons \
   "should we set a mask grid function (or functions) for all horizons?"
 {
-} "false"
+} false
 
 Boolean set_mask_for_individual_horizon[101] \
   "should we set a mask grid function (or functions) for *this* horizon?"
 {
-} "false"
+} false
 
 #
 # If we set a mask, we do so based on partitioning the Cactus grid into
@@ -576,7 +600,7 @@ Boolean mask_is_noshrink						\
   "should we prevent the inside & buffer regions from ever shrinking?"	\
   STEERABLE=ALWAYS
 {
-} "true"
+} true
 
 #
 # If we're using mesh refinement (eg Carpet) it's possible to find a
@@ -628,13 +652,13 @@ Boolean set_old_style_mask						\
    should we set an old-style (CCTK_REAL) mask gridfn?"			\
   STEERABLE = ALWAYS
 {
-} "true"
+} true
 Boolean set_new_style_mask						\
   "if we're setting a mask,						\
    should we set an new-style (CCTK_INT) mask gridfn?"			\
   STEERABLE = ALWAYS
 {
-} "false"
+} false
 
 #
 # mask info for  mask_type = "old-style (CCTK_REAL)"
@@ -728,7 +752,7 @@ int warn_level__point_outside__subsequent \
 
 # the Cactus configure process didn't find a finite() function,
 # so we're skipping the "check that the geometry is finite" tests
-# even though the user set  check_that_geometry_is_finite = "true"
+# even though the user set  check_that_geometry_is_finite = true
 int warn_level__skipping_finite_check \
   "warning level if the user sets  check_that_geometry_is_finite \
    but the Cactus configure process doesn't find a  finite()  function \
@@ -1035,9 +1059,9 @@ real ILUCG__error_tolerance "error tolerance for conjugate gradient iteration"
 Boolean ILUCG__limit_CG_iterations \
   "should we limit the maximum number of conjugate gradient iterations allowed?"
 {
-"false"	:: "no limit on CG iterations"
-"true"	:: "limit to Neqns CG iterations"
-} "true"
+false	:: "no limit on CG iterations"
+true	:: "limit to Neqns CG iterations"
+} true
 
 # extra parameters for
 #   Jacobian_store_solve_method == "row-oriented sparse matrix/UMFPACK"
@@ -1077,7 +1101,7 @@ real Jacobian_perturbation_amplitude \
 Boolean test_all_Jacobian_compute_methods \
   "should we test all Jacobian computation methods, or just NP?"
 {
-} "true"
+} true
 
 ################################################################################
 
@@ -1142,7 +1166,7 @@ Boolean hardwire_Schwarzschild_EF_geometry \
   "should we hard-wire the geometry to Schwarzschild/EF instead of \
    interpolating from the Cactus grid?"
 {
-} "false"
+} false
 
 real geometry__Schwarzschild_EF__mass "mass of Schwarzschild BH"
 {
@@ -1202,11 +1226,11 @@ real geometry__Schwarzschild_EF__Delta_xyz \
 Boolean check_that_h_is_finite \
   "should we check that horizon shape function h is finite?"
 {
-} "true"
+} true
 Boolean check_that_geometry_is_finite \
   "should we check the interpolated geometry variables are finite?"
 {
-} "true"
+} true
 
 ################################################################################
 
@@ -1333,6 +1357,6 @@ keyword integral_method "how do we compute integrals over the horizon?"
 ##  "should we perturb at *all* points in the y patch, or just those with the \
 ##   iperp which is (supposedly) involved in the interpatch interpolation?"
 ##{
-##} "true"
+##} true
 ##
 ##################################################################################