Merge pull request #2828 from boutproject/next-options-bool

More consistent parsing of boolean inputs (Breaking change)

CHANGELOG.md

@@ -6,6 +6,8 @@
 ### Breaking changes
 
 - The autotools `./configure` build system has been removed
+- Parsing of booleans has changed [\#2828][https://github.com/boutproject/BOUT-dev/pull/2828] ([bendudson][https://github.com/bendudson]).
+  See the [manual page](https://bout-dev.readthedocs.io/en/stable/user_docs/bout_options.html#boolean-expressions) for details.
 
 
 ## [v5.1.0](https://github.com/boutproject/BOUT-dev/tree/v5.1.0)

include/bout/sys/expressionparser.hxx

@@ -3,9 +3,9 @@
  *
  * Parses strings containing expressions, returning a tree of generators
  *
- * Copyright 2010 B.D.Dudson, S.Farley, M.V.Umansky, X.Q.Xu
+ * Copyright 2010-2024 BOUT++ contributors
  *
- * Contact: Ben Dudson, [email protected]
+ * Contact: Ben Dudson, [email protected]
  *
  * This file is part of BOUT++.
  *
@@ -24,8 +24,8 @@
  *
  **************************************************************************/
 
-#ifndef __EXPRESSION_PARSER_H__
-#define __EXPRESSION_PARSER_H__
+#ifndef EXPRESSION_PARSER_H
+#define EXPRESSION_PARSER_H
 
 #include "bout/format.hxx"
 #include "bout/unused.hxx"
@@ -158,7 +158,7 @@ protected:
   /// Characters which cannot be used in symbols without escaping;
   /// all other allowed. In addition, whitespace cannot be used.
   /// Adding a binary operator adds its symbol to this string
-  std::string reserved_chars = "+-*/^[](){},=";
+  std::string reserved_chars = "+-*/^[](){},=!";
 
 private:
   std::map<std::string, FieldGeneratorPtr> gen; ///< Generators, addressed by name
@@ -260,4 +260,4 @@ private:
   std::string message;
 };
 
-#endif // __EXPRESSION_PARSER_H__
+#endif // EXPRESSION_PARSER_H

include/bout/sys/generator_context.hxx

@@ -28,10 +28,7 @@ public:
   Context(int ix, int iy, int iz, CELL_LOC loc, Mesh* msh, BoutReal t);
 
   /// If constructed without parameters, contains no values (null).
-  /// Requesting x,y,z or t should throw an exception
-  ///
-  /// NOTE: For backward compatibility, all locations are set to zero.
-  /// This should be changed in a future release.
+  /// Requesting x,y,z or t throws an exception
   Context() = default;
 
   /// The location on the boundary
@@ -60,7 +57,13 @@ public:
   }
 
   /// Retrieve a value previously set
-  BoutReal get(const std::string& name) const { return parameters.at(name); }
+  BoutReal get(const std::string& name) const {
+    auto it = parameters.find(name);
+    if (it != parameters.end()) {
+      return it->second;
+    }
+    throw BoutException("Generator context doesn't contain '{:s}'", name);
+  }
 
   /// Get the mesh for this context (position)
   /// If the mesh is null this will throw a BoutException (if CHECK >= 1)
@@ -73,8 +76,7 @@ private:
   Mesh* localmesh{nullptr}; ///< The mesh on which the position is defined
 
   /// Contains user-set values which can be set and retrieved
-  std::map<std::string, BoutReal> parameters{
-      {"x", 0.0}, {"y", 0.0}, {"z", 0.0}, {"t", 0.0}};
+  std::map<std::string, BoutReal> parameters{};
 };
 
 } // namespace generator

manual/sphinx/user_docs/bout_options.rst

@@ -48,9 +48,10 @@ name in square brackets.
 
 Option names can contain almost any character except ’=’ and ’:’,
 including unicode.  If they start with a number or ``.``, contain
-arithmetic symbols (``+-*/^``), brackets (``(){}[]``), equality
-(``=``), whitespace or comma ``,``, then these will need to be escaped
-in expressions. See below for how this is done.
+arithmetic/boolean operator symbols (``+-*/^&|!<>``), brackets
+(``(){}[]``), equality (``=``), whitespace or comma ``,``, then these
+will need to be escaped in expressions. See below for how this is
+done.
 
 Subsections can also be used, separated by colons ’:’, e.g.
 
@@ -87,6 +88,13 @@ operators, with the usual precedence rules. In addition to ``π``,
 expressions can use predefined variables ``x``, ``y``, ``z`` and ``t``
 to refer to the spatial and time coordinates (for definitions of the values
 these variables take see :ref:`sec-expressions`).
+
+.. note:: The variables ``x``, ``y``, ``z`` should only be defined
+   when reading a 3D field; ``t`` should only be defined when reading
+   a time-dependent value. Earlier BOUT++ versions (v5.1.0 and earler)
+   defined all of these to be 0 by default e.g. when reading scalar
+   inputs.
+
 A number of functions are defined, listed in table
 :numref:`tab-initexprfunc`. One slightly unusual feature (borrowed from `Julia <https://julialang.org/>`_)
 is that if a number comes before a symbol or an opening bracket (``(``)
@@ -109,11 +117,11 @@ The convention is the same as in `Python <https://www.python.org/>`_:
 If brackets are not balanced (closed) then the expression continues on the next line.
 
 All expressions are calculated in floating point and then converted to
-an integer if needed when read inside BOUT++. The conversion is done by rounding
-to the nearest integer, but throws an error if the floating point
-value is not within :math:`1e-3` of an integer. This is to minimise
-unexpected behaviour. If you want to round any result to an integer,
-use the ``round`` function:
+an integer (or boolean) if needed when read inside BOUT++. The
+conversion is done by rounding to the nearest integer, but throws an
+error if the floating point value is not within :math:`1e-3` of an
+integer. This is to minimise unexpected behaviour. If you want to
+round any result to an integer, use the ``round`` function:
 
 .. code-block:: cfg
 
@@ -125,6 +133,43 @@ number, since the type is determined by how it is used.
 
 Have a look through the examples to see how the options are used.
 
+Boolean expressions
+~~~~~~~~~~~~~~~~~~~
+
+Boolean values must be "true", "false", "True", "False", "1" or
+"0". All lowercase ("true"/"false") is preferred, but the uppercase
+versions are allowed to support Python string conversions. Booleans
+can be combined into expressions using binary operators `&` (logical
+AND), `|` (logical OR), and unary operator `!` (logical NOT). For
+example "true & false" evaluates to `false`; "!false" evaluates to
+`true`.  Like real values and integers, boolean expressions can refer
+to other variables:
+
+.. code-block:: cfg
+
+   switch = true
+   other_switch = !switch
+
+Boolean expressions can be formed by comparing real values using
+`>` and `<` comparison operators:
+
+.. code-block:: cfg
+
+   value = 3.2
+   is_true = value > 3
+   is_false = value < 2
+
+.. note::
+   Previous BOUT++ versions (v5.1.0 and earlier) were case
+   insensitive when reading boolean values, so would read "True" or
+   "yEs" as `true`, and "False" or "No" as `false`. These earlier
+   versions did not allow boolean expressions.
+
+Internally, booleans are evaluated as real values, with `true` being 1
+and `false` being 0. Logical operators (`&`, `|`, `!`) check that
+their left and right arguments are either close to 0 or close to 1
+(like integers, "close to" is within 1e-3).
+
 Special symbols in Option names
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

src/field/field_factory.cxx

@@ -93,8 +93,20 @@ FieldFactory::FieldFactory(Mesh* localmesh, Options* opt)
   // Note: don't use 'options' here because 'options' is a 'const Options*'
   // pointer, so this would fail if the "input" section is not present.
   Options& nonconst_options{opt == nullptr ? Options::root() : *opt};
-  transform_from_field_aligned =
-      nonconst_options["input"]["transform_from_field_aligned"].withDefault(true);
+
+  // Convert from string, or FieldFactory is used to parse the string
+  auto str =
+      nonconst_options["input"]["transform_from_field_aligned"].withDefault<std::string>(
+          "true");
+  if ((str == "true") or (str == "True")) {
+    transform_from_field_aligned = true;
+  } else if ((str == "false") or (str == "False")) {
+    transform_from_field_aligned = false;
+  } else {
+    throw ParseException(
+        "Invalid boolean given as input:transform_from_field_aligned: '{:s}'",
+        nonconst_options["input"]["transform_from_field_aligned"].as<std::string>());
+  }
 
   // Convert using stoi rather than Options, or a FieldFactory is used to parse
   // the string, leading to infinite loop.
@@ -114,6 +126,14 @@ FieldFactory::FieldFactory(Mesh* localmesh, Options* opt)
   addGenerator("pi", std::make_shared<FieldValue>(PI));
   addGenerator("π", std::make_shared<FieldValue>(PI));
 
+  // Boolean values
+  addGenerator("true", std::make_shared<FieldValue>(1));
+  addGenerator("false", std::make_shared<FieldValue>(0));
+
+  // Python converts booleans to True/False
+  addGenerator("True", std::make_shared<FieldValue>(1));
+  addGenerator("False", std::make_shared<FieldValue>(0));
+
   // Some standard functions
   addGenerator("sin", std::make_shared<FieldGenOneArg<sin>>(nullptr, "sin"));
   addGenerator("cos", std::make_shared<FieldGenOneArg<cos>>(nullptr, "cos"));

src/sys/expressionparser.cxx

@@ -184,10 +184,29 @@ FieldGeneratorPtr FieldBinary::clone(const list<FieldGeneratorPtr> args) {
   return std::make_shared<FieldBinary>(args.front(), args.back(), op);
 }
 
+/// Convert a real value to a Boolean
+/// Throw exception if `rval` isn't close to 0 or 1
+bool toBool(BoutReal rval) {
+  int ival = ROUND(rval);
+  if ((fabs(rval - static_cast<BoutReal>(ival)) > 1e-3) or (ival < 0) or (ival > 1)) {
+    throw BoutException(_("Boolean operator argument {:e} is not a bool"), rval);
+  }
+  return ival == 1;
+}
+
 BoutReal FieldBinary::generate(const Context& ctx) {
   BoutReal lval = lhs->generate(ctx);
   BoutReal rval = rhs->generate(ctx);
+
   switch (op) {
+  case '|': // Logical OR
+    return (toBool(lval) or toBool(rval)) ? 1.0 : 0.0;
+  case '&': // Logical AND
+    return (toBool(lval) and toBool(rval)) ? 1.0 : 0.0;
+  case '>': // Comparison
+    return (lval > rval) ? 1.0 : 0.0;
+  case '<':
+    return (lval < rval) ? 1.0 : 0.0;
   case '+':
     return lval + rval;
   case '-':
@@ -203,10 +222,30 @@ BoutReal FieldBinary::generate(const Context& ctx) {
   throw ParseException("Unknown binary operator '{:c}'", op);
 }
 
+class LogicalNot : public FieldGenerator {
+public:
+  /// Logically negate a boolean expression
+  LogicalNot(FieldGeneratorPtr expr) : expr(expr) {}
+
+  /// Evaluate expression, check it's a bool, and return 1 or 0
+  double generate(const Context& ctx) override {
+    return toBool(expr->generate(ctx)) ? 0.0 : 1.0;
+  }
+
+  std::string str() const override { return "!"s + expr->str(); }
+
+private:
+  FieldGeneratorPtr expr;
+};
+
 /////////////////////////////////////////////
 
 ExpressionParser::ExpressionParser() {
   // Add standard binary operations
+  addBinaryOp('|', std::make_shared<FieldBinary>(nullptr, nullptr, '|'), 3);
+  addBinaryOp('&', std::make_shared<FieldBinary>(nullptr, nullptr, '&'), 5);
+  addBinaryOp('<', std::make_shared<FieldBinary>(nullptr, nullptr, '<'), 7);
+  addBinaryOp('>', std::make_shared<FieldBinary>(nullptr, nullptr, '>'), 7);
   addBinaryOp('+', std::make_shared<FieldBinary>(nullptr, nullptr, '+'), 10);
   addBinaryOp('-', std::make_shared<FieldBinary>(nullptr, nullptr, '-'), 10);
   addBinaryOp('*', std::make_shared<FieldBinary>(nullptr, nullptr, '*'), 20);
@@ -482,6 +521,11 @@ FieldGeneratorPtr ExpressionParser::parsePrimary(LexInfo& lex) const {
     // Don't eat the minus, and return an implicit zero
     return std::make_shared<FieldValue>(0.0);
   }
+  case '!': {
+    // Logical not
+    lex.nextToken(); // Eat '!'
+    return std::make_shared<LogicalNot>(parsePrimary(lex));
+  }
   case '(': {
     return parseParenExpr(lex);
   }

src/sys/options.cxx

@@ -433,19 +433,16 @@ bool Options::as<bool>(const bool& UNUSED(similar_to)) const {
     result = bout::utils::get<bool>(value);
 
   } else if (bout::utils::holds_alternative<std::string>(value)) {
-    // case-insensitve check, so convert string to lower case
-    const auto strvalue = lowercase(bout::utils::get<std::string>(value));
-
-    if ((strvalue == "y") or (strvalue == "yes") or (strvalue == "t")
-        or (strvalue == "true") or (strvalue == "1")) {
-      result = true;
-    } else if ((strvalue == "n") or (strvalue == "no") or (strvalue == "f")
-               or (strvalue == "false") or (strvalue == "0")) {
-      result = false;
-    } else {
-      throw BoutException(_("\tOption '{:s}': Boolean expected. Got '{:s}'\n"), full_name,
-                          strvalue);
+    // Parse as floating point because that's the only type the parser understands
+    BoutReal rval = parseExpression(value, this, "bool", full_name);
+
+    // Check that the result is either close to 1 (true) or close to 0 (false)
+    int ival = ROUND(rval);
+    if ((fabs(rval - static_cast<BoutReal>(ival)) > 1e-3) or (ival < 0) or (ival > 1)) {
+      throw BoutException(_("Value for option {:s} = {:e} is not a bool"), full_name,
+                          rval);
     }
+    result = ival == 1;
   } else {
     throw BoutException(_("Value for option {:s} cannot be converted to a bool"),
                         full_name);

src/sys/options/options_ini.cxx

@@ -189,7 +189,7 @@ void OptionINI::parse(const string& buffer, string& key, string& value) {
     // Just set a flag to true
     // e.g. "restart" or "append" on command line
     key = buffer;
-    value = string("TRUE");
+    value = string("true");
     return;
   }
 

tests/integrated/test-boutpp/collect-staggered/data/BOUT.inp

@@ -2,7 +2,7 @@ nout = 10
 timestep = 0.1
 
 [mesh]
-staggergrids = True
+staggergrids = true
 n = 1
 nx = n+2*MXG
 ny = 16

tests/integrated/test-boutpp/collect/input/BOUT.inp

@@ -5,7 +5,7 @@ MXG = 2
 MYG = 2
 
 [mesh]
-staggergrids = True
+staggergrids = true
 n = 1
 nx = n+2*MXG
 ny = n

tests/integrated/test-boutpp/legacy-model/data/BOUT.inp

@@ -2,7 +2,7 @@ nout = 10
 timestep = 0.1
 
 [mesh]
-staggergrids = True
+staggergrids = true
 n = 1
 nx = n+2*MXG
 ny = n

tests/integrated/test-boutpp/mms-ddz/data/BOUT.inp

@@ -5,7 +5,7 @@ MXG = 2
 MYG = 2
 
 [mesh]
-staggergrids = True
+staggergrids = true
 n = 1
 nx = n+2*MXG
 ny = n

tests/integrated/test-twistshift-staggered/test-twistshift.cxx

@@ -4,11 +4,11 @@
 int main(int argc, char** argv) {
   BoutInitialise(argc, argv);
 
-  Field3D test = FieldFactory::get()->create3D("test", nullptr, nullptr, CELL_YLOW);
+  using bout::globals::mesh;
 
-  Field3D test_aligned = toFieldAligned(test);
+  Field3D test = FieldFactory::get()->create3D("test", nullptr, mesh, CELL_YLOW);
 
-  using bout::globals::mesh;
+  Field3D test_aligned = toFieldAligned(test);
 
   // zero guard cells to check that communication is doing something
   for (int x = 0; x < mesh->LocalNx; x++) {
@@ -25,7 +25,7 @@ int main(int argc, char** argv) {
   mesh->communicate(test_aligned);
 
   Options::root()["check"] =
-      FieldFactory::get()->create3D("check", nullptr, nullptr, CELL_YLOW);
+      FieldFactory::get()->create3D("check", nullptr, mesh, CELL_YLOW);
 
   Options::root()["test"] = test;
   Options::root()["test_aligned"] = test_aligned;