r.mapcalc: deprecate -s flag, use autoseeding by default

gui/wxpython/modules/mcalc_builder.py

@@ -288,17 +288,13 @@ def __init__(
             UserSettings.Get(group="cmd", key="overwrite", subkey="enabled")
         )
 
-        self.randomSeed = wx.CheckBox(
-            parent=self.panel, label=_("Generate random seed for rand()")
+        self.randomSeedStaticText = StaticText(
+            parent=self.panel, label=_("Random seed:")
         )
-        self.randomSeedStaticText = StaticText(parent=self.panel, label=_("Seed:"))
         self.randomSeedText = TextCtrl(
             parent=self.panel, size=(100, -1), validator=IntegerValidator()
         )
         self.randomSeedText.SetToolTip(_("Integer seed for rand() function"))
-        self.randomSeed.SetValue(True)
-        self.randomSeedStaticText.Disable()
-        self.randomSeedText.Disable()
 
         self.addbox = wx.CheckBox(
             parent=self.panel,
@@ -331,8 +327,6 @@ def __init__(
         self.newmaptxt.Bind(wx.EVT_TEXT, self.OnUpdateStatusBar)
         self.text_mcalc.Bind(wx.EVT_TEXT, self.OnUpdateStatusBar)
         self.overwrite.Bind(wx.EVT_CHECKBOX, self.OnUpdateStatusBar)
-        self.randomSeed.Bind(wx.EVT_CHECKBOX, self.OnUpdateStatusBar)
-        self.randomSeed.Bind(wx.EVT_CHECKBOX, self.OnSeedFlag)
         self.randomSeedText.Bind(wx.EVT_TEXT, self.OnUpdateStatusBar)
 
         # bind closing to ESC
@@ -449,12 +443,6 @@ def _layout(self):
         sizer.Add(buttonSizer4, proportion=0, flag=wx.ALIGN_RIGHT | wx.ALL, border=3)
 
         randomSizer = wx.BoxSizer(wx.HORIZONTAL)
-        randomSizer.Add(
-            self.randomSeed,
-            proportion=0,
-            flag=wx.RIGHT | wx.ALIGN_CENTER_VERTICAL,
-            border=20,
-        )
         randomSizer.Add(
             self.randomSeedStaticText,
             proportion=0,
@@ -565,13 +553,6 @@ def OnUpdateStatusBar(self, event):
         self.SetStatusText(command)
         event.Skip()
 
-    def OnSeedFlag(self, event):
-        checked = self.randomSeed.IsChecked()
-        self.randomSeedText.Enable(not checked)
-        self.randomSeedStaticText.Enable(not checked)
-
-        event.Skip()
-
     def _getCommand(self):
         """Returns entire command as string."""
         expr = self.text_mcalc.GetValue().strip().replace("\n", " ")
@@ -581,18 +562,14 @@ def _getCommand(self):
         overwrite = ""
         if self.overwrite.IsChecked():
             overwrite = " --overwrite"
-        seed_flag = seed = ""
-        if re.search(pattern=r"rand *\(.+\)", string=expr):
-            if self.randomSeed.IsChecked():
-                seed_flag = " -s"
-            else:
-                seed = " seed={val}".format(val=self.randomSeedText.GetValue().strip())
+        seed = ""
+        if self.randomSeedText.GetValue():
+            seed = " seed={val}".format(val=self.randomSeedText.GetValue().strip())
 
-        return '{cmd} expression="{new} = {expr}"{seed}{seed_flag}{overwrite}'.format(
+        return '{cmd} expression="{new} = {expr}"{seed}{overwrite}'.format(
             cmd=cmd,
             expr=expr,
             new=self.newmaptxt.GetValue(),
-            seed_flag=seed_flag,
             seed=seed,
             overwrite=overwrite,
         )
@@ -663,16 +640,9 @@ def OnMCalcRun(self, event):
             )
             return
 
-        seed_flag = seed = None
-        if re.search(pattern=r"rand *\(.+\)", string=expr):
-            if self.randomSeed.IsChecked():
-                seed_flag = "-s"
-            else:
-                seed = self.randomSeedText.GetValue().strip()
+        seed = self.randomSeedText.GetValue().strip()
         if self.log:
             cmd = [self.cmd]
-            if seed_flag:
-                cmd.append("-s")
             if seed:
                 cmd.append("seed={val}".format(val=seed))
             if self.overwrite.IsChecked():
@@ -684,8 +654,6 @@ def OnMCalcRun(self, event):
         else:
             overwrite = bool(self.overwrite.IsChecked())
             params = {"expression": "%s=%s" % (name, expr), "overwrite": overwrite}
-            if seed_flag:
-                params["flags"] = "s"
             if seed:
                 params["seed"] = seed
 

raster/r.colors.out/r3.colors.out.md

@@ -18,7 +18,7 @@ which includes the following options:
 === "Command line"
 
     ```sh
-    r3.mapcalc "random = rand(1, 5)" -s
+    r3.mapcalc "random = rand(1, 5)"
     r3.colors map=random color=gyr
     r3.colors.out map=random
     ```
@@ -38,7 +38,7 @@ which includes the following options:
     ```python
     import grass.script as gs
 
-    gs.run_command("r3.mapcalc", expression="random = rand(1, 5)", flags="s")
+    gs.run_command("r3.mapcalc", expression="random = rand(1, 5)")
     gs.run_command("r3.colors", map="random", color="gyr")
     colors = gs.parse_command("r3.colors.out", map="random", format="json")
     ```

raster/r.mapcalc/main.c

@@ -19,6 +19,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <stdbool.h>
 
 #include <grass/glocale.h>
 
@@ -57,7 +58,41 @@ static expr_list *parse_file(const char *filename)
     return res;
 }
 
-/****************************************************************************/
+static bool has_rand_expr(const expression *e)
+{
+    if (!e)
+        return 0;
+
+    switch (e->type) {
+    case expr_type_function:
+        if (strcmp(e->data.func.name, "rand") == 0)
+            return 1;
+        // args is 1-indexed (likely from yacc parser conventions)
+        for (int i = 1; i <= e->data.func.argc; i++) {
+            if (has_rand_expr(e->data.func.args[i]))
+                return 1;
+        }
+        return 0;
+
+    case expr_type_binding:
+        return has_rand_expr(e->data.bind.val);
+
+    case expr_type_variable:
+        return has_rand_expr(e->data.var.bind);
+
+    default:
+        return 0;
+    }
+}
+
+static bool expr_list_has_rand(const expr_list *list)
+{
+    for (; list; list = list->next) {
+        if (has_rand_expr(list->exp))
+            return 1;
+    }
+    return 0;
+}
 
 int main(int argc, char **argv)
 {
@@ -112,8 +147,11 @@ int main(int argc, char **argv)
 
     random = G_define_flag();
     random->key = 's';
+    random->label =
+        _("Generate random seed (result is non-deterministic) [deprecated]");
     random->description =
-        _("Generate random seed (result is non-deterministic)");
+        _("This flag is deprecated and will be removed in a future release. "
+          "Seeding is automatic or use parameter seed.");
 
     describe = G_define_flag();
     describe->key = 'l';
@@ -153,17 +191,24 @@ int main(int argc, char **argv)
     if (!result)
         G_fatal_error(_("parse error"));
 
+    bool has_rand = expr_list_has_rand(result);
     if (seed->answer) {
         seed_value = atol(seed->answer);
         G_srand48(seed_value);
         seeded = 1;
         G_debug(3, "Read random seed from seed=: %ld", seed_value);
     }
-
-    if (random->answer) {
-        seed_value = G_srand48_auto();
-        seeded = 1;
-        G_debug(3, "Generated random seed (-s): %ld", seed_value);
+    else {
+        if (has_rand) {
+            seed_value = G_srand48_auto();
+            seeded = 1;
+            G_debug(3, "Automatically generated random seed: %ld", seed_value);
+        }
+        if (random->answer) {
+            G_verbose_message(_("Flag 's' is deprecated and will be removed in "
+                                "a future release. "
+                                "Seeding is automatic or use parameter seed."));
+        }
     }
 
     /* Set the global variable of the region setup approach */
@@ -200,11 +245,11 @@ int main(int argc, char **argv)
         nprocs->answer = "1";
         G_verbose_message(_("r3.mapcalc does not support parallel execution."));
     }
-    else if ((seeded) && (threads != 1)) {
+    else if ((threads != 1) && (has_rand)) {
         threads = 1;
         nprocs->answer = "1";
         G_verbose_message(
-            _("Parallel execution is not supported for random seed."));
+            _("Parallel execution is not supported with rand() function"));
     }
 
     /* Ensure the proper number of threads is assigned */

raster/r.mapcalc/r.mapcalc.md

@@ -762,23 +762,18 @@ g.rename raster=newmap,oldmap
 ### Random number generator initialization
 
 The pseudo-random number generator used by the rand() function can be
-initialised to a specific value using the **seed** option. This can be
+initialised to a specific value using the **seed** parameter. This can be
 used to replicate a previous calculation.
 
-Alternatively, it can be initialised from the system time and the PID
-using the **-s** flag. This should result in a different seed being used
-each time.
+If rand() function is used and no seed is given,
+*r.mapcalc* will generate a seed, resulting in a different result for
+each run.
 
 In either case, the seed will be written to the map's history, and can
 be seen using *r.info*.
 
 If you want other people to be able to verify your results, it's
-preferable to use the **seed** option to supply a seed which is either
-specified in the script or generated from a deterministic process such
-as a pseudo-random number generator given an explicit seed.
-
-Note that the rand() function will generate a fatal error if neither the
-**seed** option nor the **-s** flag are given.
+preferable to use the **seed** parameter.
 
 ## EXAMPLES
 
@@ -819,8 +814,7 @@ To change all values below 5 to NULL, keep 5 otherwise:
 newmap = if(map<5, null(), 5)
 ```
 
-To create a map with random values in a defined range (needs either the
-usage of **-s** flag or the *seed* parameter). The precision of the
+To create a map with random values in a defined range. The precision of the
 input values determines the output precision (the resulting [raster map
 type](rasterintro.md#raster-format)):
 

raster/r.mapcalc/r3.mapcalc.md

@@ -551,23 +551,18 @@ they are not NULL. See *[r3.mask](r3.mask.md)* for details.
 ### Random number generator initialization
 
 The pseudo-random number generator used by the rand() function can be
-initialised to a specific value using the **seed** option. This can be
+initialised to a specific value using the **seed** parameter. This can be
 used to replicate a previous calculation.
 
-Alternatively, it can be initialised from the system time and the PID
-using the **-r** flag. This should result in a different seed being used
-each time.
+If rand() function is used and no seed is given,
+*r3.mapcalc* will generate a seed, resulting in a different result for
+each run.
 
 In either case, the seed will be written to the map's history, and can
 be seen using *r.info*.
 
 If you want other people to be able to verify your results, it's
-preferable to use the **seed** option to supply a seed which is either
-specified in the script or generated from a deterministic process such
-as a pseudo-random number generator given an explicit seed.
-
-Note that the rand() function will generate a fatal error if neither the
-**seed** option nor the **-s** flag are given.
+preferable to use the **seed** parameter.
 
 ## EXAMPLES
 

raster/r.mapcalc/tests/r_mapcalc_nprocs_test.py

@@ -43,8 +43,7 @@ def test_rand_no_explicit_seed_setting(session_in_mapset, nprocs):
         session=session_in_mapset, consistent_return_value=True, errors="ignore"
     )
     result = tools.r_mapcalc(expression="test = rand(-15.0, 5.0)", nprocs=nprocs)
-    assert result.returncode == 1
-    assert "not seeded" in result.stderr
+    assert result.returncode == 0
 
 
 @pytest.mark.parametrize("nprocs", [0, 1, 4])

raster/r.mapcalc/testsuite/test_r3_mapcalc.py

@@ -29,15 +29,15 @@ def tearDownClass(cls):
 
     def test_difference_of_the_same_map_double(self):
         """Test zero difference of map with itself"""
-        self.runModule("r3.mapcalc", flags="s", expression="a = rand(1.0, 200)")
+        self.runModule("r3.mapcalc", expression="a = rand(1.0, 200)")
         self.to_remove.append("a")
         self.assertModule("r3.mapcalc", expression="diff_a_a = a - a")
         self.to_remove.append("diff_a_a")
         self.assertRaster3dMinMax("diff_a_a", refmin=0, refmax=0)
 
     def test_difference_of_the_same_map_float(self):
         """Test zero difference of map with itself"""
-        self.runModule("r3.mapcalc", flags="s", expression="af = rand(float(1), 200)")
+        self.runModule("r3.mapcalc", expression="af = rand(float(1), 200)")
         self.to_remove.append("af")
         self.assertModule("r3.mapcalc", expression="diff_af_af = af - af")
         self.to_remove.append("diff_af_af")

raster/r.mapcalc/testsuite/test_r_mapcalc.py

@@ -91,15 +91,6 @@ def test_seed_not_required(self):
         self.assertModule("r.mapcalc", expression="nonrand_cell = 200")
         self.to_remove.append("nonrand_cell")
 
-    def test_seed_required(self):
-        """Test that seed is required when rand() is used
-
-        This test can, and probably should, generate an error message.
-        """
-        self.assertModuleFail("r.mapcalc", expression="rand_x = rand(1, 200)")
-        # TODO: assert map not exists but it would be handy here
-        # TODO: test that error message was generated
-
     def test_seed_cell(self):
         """Test given seed with CELL against reference map"""
         seed = 500
@@ -153,14 +144,10 @@ def test_seed_fcell(self):
         self.rinfo_contains_number("rand_fcell", seed)
 
     def test_auto_seed(self):
-        """Test that two runs with -s does not give same maps"""
-        self.assertModule(
-            "r.mapcalc", flags="s", expression="rand_auto_1 = rand(1., 2)"
-        )
+        """Test that two runs do not give same maps"""
+        self.assertModule("r.mapcalc", expression="rand_auto_1 = rand(1., 2)")
         self.to_remove.append("rand_auto_1")
-        self.assertModule(
-            "r.mapcalc", flags="s", expression="rand_auto_2 = rand(1., 2)"
-        )
+        self.assertModule("r.mapcalc", expression="rand_auto_2 = rand(1., 2)")
         self.to_remove.append("rand_auto_2")
         self.assertRastersDifference(
             "rand_auto_1",
@@ -197,23 +184,23 @@ def tearDownClass(cls):
 
     def test_difference_of_the_same_map_double(self):
         """Test zero difference of map with itself"""
-        self.runModule("r.mapcalc", flags="s", expression="a = rand(1.0, 200)")
+        self.runModule("r.mapcalc", expression="a = rand(1.0, 200)")
         self.to_remove.append("a")
         self.assertModule("r.mapcalc", expression="diff_a_a = a - a")
         self.to_remove.append("diff_a_a")
         self.assertRasterMinMax("diff_a_a", refmin=0, refmax=0)
 
     def test_difference_of_the_same_map_float(self):
         """Test zero difference of map with itself"""
-        self.runModule("r.mapcalc", flags="s", expression="af = rand(float(1), 200)")
+        self.runModule("r.mapcalc", expression="af = rand(float(1), 200)")
         self.to_remove.append("af")
         self.assertModule("r.mapcalc", expression="diff_af_af = af - af")
         self.to_remove.append("diff_af_af")
         self.assertRasterMinMax("diff_af_af", refmin=0, refmax=0)
 
     def test_difference_of_the_same_map_int(self):
         """Test zero difference of map with itself"""
-        self.runModule("r.mapcalc", flags="s", expression="ai = rand(1, 200)")
+        self.runModule("r.mapcalc", expression="ai = rand(1, 200)")
         self.to_remove.append("ai")
         self.assertModule("r.mapcalc", expression="diff_ai_ai = ai - ai")
         self.to_remove.append("diff_ai_ai")