Update add-vars.md#4332
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4332 +/- ##
==========================================
- Coverage 48.86% 48.64% -0.23%
==========================================
Files 151 151
Lines 29417 29675 +258
==========================================
+ Hits 14374 14434 +60
- Misses 15043 15241 +198 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Requesting a couple of reviews to ensure these docs make sense and are sufficiently detailed |
0aba1cf to
d1dd370
Compare
| my_new_blanket_variable = 1.0 | ||
| ``` | ||
|
|
||
| ----------------- |
There was a problem hiding this comment.
Do we need a bit here about how the value of an input variable is unchanged throughout a run ?
There was a problem hiding this comment.
This point is made at least twice in the document, I think that should be enough
There was a problem hiding this comment.
Just read through the whole page again and this point isn't made until the 'add an iteration variable' section - I think it'd be better to include it in the 'add an input ' section and then can shorten or delete the sentence in the iteration variable section
There was a problem hiding this comment.
At the top of the page:
It is advised that a variable is either used to define a particular PROCESS run (input variable, iteration variable, or scan variable) or mutated within a PROCESS run (output variable or intermediate variable). Mixing the two classes of variable (e.g. having a variable that can be input but is also mutated within a model) will lead to confusing, dangerous, and incorrect results.
| ## Add a constraint equation | ||
|
|
||
| Constraint equations are added to *PROCESS* in the `process/core/solver/constraints.py` file. They are registered with the `ConstraintManager` whenever the application is run. Each equation has a unique name that is currently an integer, however upgrades to the input file format in the future will allow arbitrary hashable constraint names. | ||
| Constraint equations are added to PROCESS in the `process/core/solver/constraints.py` file. They are registered with the `ConstraintManager` whenever the application is run. Each equation has a unique name that is currently an integer, however upgrades to the input file format in the future will allow arbitrary hashable constraint names. |
There was a problem hiding this comment.
Will updates to the input file format mean changes for more things in this page other than just constraints? If so, probably worth a general note at the start of this page? Might not though, just thought I'd ask!
There was a problem hiding this comment.
Tbh that information is probably not necessary and I will remove it
Co-authored-by: clmould <86794332+clmould@users.noreply.github.com> Co-authored-by: Christopher Ashe <91618944+chris-ashe@users.noreply.github.com>
Co-authored-by: clmould <86794332+clmould@users.noreply.github.com>
a18ef77 to
3af3782
Compare
| @@ -153,20 +174,26 @@ The arguments to the `register_constraint` function are: | |||
|
|
|||
| - Name (again, currently an integer) | |||
| - Unit (for output reporting purposes) | |||
| - Symbol (e.g. =, >=, <=. Again, for output reporting purposes) | |||
| - Symbol (e.g. `=`, `>=`, `<=`. Again, for output reporting purposes) | |||
There was a problem hiding this comment.
| - Symbol (e.g. `=`, `>=`, `<=`. Again, for output reporting purposes) |
think github might've messed up my suggestion ! not sure why it added it instead of overwriting !
chris-ashe
left a comment
There was a problem hiding this comment.
@clmould comments have covered my comments
clmould
left a comment
There was a problem hiding this comment.
1 q and 1 small reordering request/suggestion, but otherwise happy with this !
| my_new_blanket_variable = 1.0 | ||
| ``` | ||
|
|
||
| ----------------- |
There was a problem hiding this comment.
Just read through the whole page again and this point isn't made until the 'add an iteration variable' section - I think it'd be better to include it in the 'add an input ' section and then can shorten or delete the sentence in the iteration variable section
| ``` | ||
| my_new_blanket_variable = 1.0 | ||
| ``` | ||
|
|
There was a problem hiding this comment.
| Note you can omit the line specifying the initial value for an input variable, and then the initial value would just be whatever that variable's default value is: in this example we could omit the `my_new_blanket_variable = 1.0` line, and `my_new_blanket_variable` would take its initial value as the default of `0.0` that we assigned [earlier](#add-a-new-variable). | |
| my_new_blanket_variable = 0.5 | ||
| ``` | ||
|
|
||
| Note you can omit the line specifying the initial value for an iteration variable, and then the initial value would just be whatever that variable's default value is: in this example we could omit the `my_new_blanket_variable = 0.5` line, and `my_new_blanket_variable` would take its initial value as the default of `0.0` that we assigned [earlier](#add-a-new-variable). |
There was a problem hiding this comment.
| Note you can omit the line specifying the initial value for an iteration variable, and then the initial value would just be whatever that variable's default value is: in this example we could omit the `my_new_blanket_variable = 0.5` line, and `my_new_blanket_variable` would take its initial value as the default of `0.0` that we assigned [earlier](#add-a-new-variable). | |
| Again, note that omitting the line specifying the initial value for an iteration variable means the initial value would be set to that variable's default value. |
This sentence could instead be deleted, but suggested a change in line with my other here in case you want to keep it, idm either way!
clmould
left a comment
There was a problem hiding this comment.
small change but happy otherwise
Co-authored-by: clmould <86794332+clmould@users.noreply.github.com>
* Update docs for adding variables, inputs, scans, FoMs, and iteration variables * Remove need to increment numerics IP variables * Correct bullet list formatting * Ensure ipeqns is maximum constraint ID * Fix cyclic import * Fix spelling/grammar errors Co-authored-by: clmould <86794332+clmould@users.noreply.github.com> Co-authored-by: Christopher Ashe <91618944+chris-ashe@users.noreply.github.com> * Clarify sections of add-vars.md * Rewording docs Co-authored-by: clmould <86794332+clmould@users.noreply.github.com> * Further clarifications * Lingering bullet * Remove duplication * Clarify default values being used * Spelling mistake Co-authored-by: clmould <86794332+clmould@users.noreply.github.com> --------- Co-authored-by: clmould <86794332+clmould@users.noreply.github.com> Co-authored-by: Christopher Ashe <91618944+chris-ashe@users.noreply.github.com>
Updated the guide for adding variables, inputs, scan vars, figures of merit, and iteration vars. They needed a refresh due to changes in the Python conversion, recent data structure changes, and enumification.
I have also reduced the need for users to manually increment some variables (was not possible for the scan variables due to circular imports).