Rename Positon -> Context, simplify class, fix things #1750
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now more general than position, can contain other keys. Renamed `position.[ch]xx` to `generator-context.[ch]xx` Simplifying the `Context` (`Position`) class, so logic of cell location is handled in constructors only.
dschwoerer
reviewed
Jun 17, 2019
| pos.setY(y); | ||
| pos.setZ(z); | ||
| return pos; | ||
| Context LegacyPosition(BoutReal x, BoutReal y, BoutReal z, BoutReal t) { |
There was a problem hiding this comment.
Shouldn't this be LegacyContext?
There was a problem hiding this comment.
Yes probably, thanks! Once the name is settled this can be modified. I'm not sure about testing legacy uses of the code though, and would prefer to test ways that the code is intended to be used.
| @@ -49,7 +45,7 @@ class BinaryGenerator : public FieldGenerator { | |||
| return std::make_shared<BinaryGenerator>(args.front(), args.back()); | |||
| } | |||
|
|
|||
| BoutReal generate(Position pos) { | |||
| BoutReal generate(const Context& pos) override { | |||
There was a problem hiding this comment.
If we rename Position to Context - should pos be renamed to con or context
There was a problem hiding this comment.
Yes probably, but that was harder to search and replace.
|
Thanks @dschwoerer . For the name I'd be happy with something else. I agree that |
Small fixes for tests
Replace `FieldSin` and similar with use of `FieldGenOneArg` Add a `std::string name` attribute to `FieldGenOneArg` and `FieldGenTwoArgs` so that a useful description can be printed.
bendudson
changed the title
Mosly introduced when changing generators to `Context`/`Position`: Neumann boundary conditions need a factor of the grid spacing to be included when generating a value. A couple of copy-paste bugs in applying Neumann boundaries to `Field2D` objects: `dx` used rather than `dy`. More tests needed on boundary conditions.
…o ynorm2-simplify
Namespacing so that the meaning and purpose of the class is clearer. Also moved `generator_context.cxx` from `mesh` into `sys` to follow structure of header file location.
Add `using bout::generator::Context` to unit tests. Rename `LegacyPosition` to `LegacyContext`
Enables more clear swiching than the H(x) Heaviside function Like the C++ functions in BOUT++, one of two expressions is evaluated, depending on the sign of the first expression.
Allows the definition of new scopes with local variables,
by setting values in the `Context` object.
One use is to simplify expressions using local variables, for
efficiency and readability e.g. ensuring that a value is positive:
```
positive_value = H(expression) * expression
```
where the expression would be evaluated twice. This can be replaced by:
```
positive_value = [value = expression]( H({value}) * {value} )
```
This mechanism can also be used to implement simple functions
```
mycosh = 0.5 * (exp({arg}) + exp(-{arg}))
```
which can be called by defining a new Context:
```
result = [arg = x*2](mycosh)
```
(sort-of a dynamic scope system)
so we could write:
```
floor = H({arg}) * {arg}
positive_value = [arg = expression](floor)
```
Some code not recompiled previously, now updated. Another test defining two variables in a `Context` object.
Long expressions can become hard to read in the options file.
This adopts the Python approach, where if there are unbalanced
brackets ('()' or '[]') then the expression continues on the next
line.
In the BOUT.inp file we can therefore write
```
value = (something
+ more
- things)
```
or define complex expression using a Context scope:
```
result = [a = expression,
b = something](
{a} + {b})
```
Enables a simple kind of loop, but one which is guaranteed to finish. Iterates a given symbol from 0 to count-1, evaluating and summing a given expression.
Using parseExpression captures the rest of the string, rather than just the next expression. Instead call parseParenExpr to stop when parentheses close. Added tests to reproduce this.
By default recursive expressions are not allowed in the input options.
What is committed here adds an input option to allow recursion:
```
[input]
max_recursion_depth = 10 # 0 = none, -1 = unlimited
```
By putting a limit on the depth we avoid the halting problem at least,
and expressions should (eventually) terminate, preferably before they
overflow the stack.
As is traditional, I present to you the Fibonacci sequence:
```
fib = where({n} - 2.5,
[n={n}-1](fib) + [n={n}-2](fib),
1)
```
so if `n` = 1 or 2 then `fib` = 1, but if `n` = 3 or above then the
recursion fun begins.
And another ill-considered language is born.
`FieldFactory` reads `input:max_recursion_depth` as an int. `Options` then creates a `FieldFactory` to parse the string to an int. This change uses `std::stoi` to do the string conversion. This means no fancy expressions in this parameter, but that should not be needed.
Documenting new features for expressions, and some examples of how to use them. Some of the existing documentation was out of date from earlier changes.
Slight efficiency improvement when repeatedly iterating over
More expressions "improvements"
d7919
changed the title
d7919
changed the title
Codacy would like several constructors to be explicit. Two of these (FieldIndirect and FieldGenOneArg) take strings, so it is useful to enable these to take `const char*` arguments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now more general than position, can contain other keys.
Renamed
position.[ch]xxtogenerator_context.[ch]xxSimplifying the
Context(Position) class, so logic of cell location is handled in constructors only. Now passed around as const references to generators.Removing some unneeded
FieldGeneratorcode.