-
Notifications
You must be signed in to change notification settings - Fork 170
Define and apply a GAMS code style #934
Copy link
Copy link
Open
Labels
discussDiscussion, design, and planningDiscussion, design, and planningenhNew features & functionalityNew features & functionalitygamsRelated to the GAMS core formulationRelated to the GAMS core formulationhelp wanted
Description
Currently we have only the following in our documentation:
message_ix/doc/contributing.rst
Lines 279 to 281 in ea06614
| - For *GAMS* code: | |
| - **Wrap** lines at 121 characters, except for inline documentation (see below). |
Working on #924, I found that the formatting of some of the existing equations made reading and understanding the code unnecessarily difficult. For example, in the following from ACTIVITY_CONSTRAINT_UP:
)
* growth of 'capital stock' from previous period
+ SUM(year_all2$( seq_period(year_all2,year) ),
CAP_NEW(node,inv_tec,year_all2)$( map_tec(node,inv_tec,year_all2) AND model_horizon(year_all2) )
+ historical_new_capacity(node,inv_tec,year_all2)
# placeholder for spillover across nodes, technologies, periods (other than immediate predecessor)
) * POWER( 1 + growth_new_capacity_lo(node,inv_tec,year) , duration_period(year) )
* 'soft' relaxation of dynamic constraints
…where are the closing parentheses for each of the three calls of the GAMS SUM() function?
In that PR, I loosely experimented with a style like the following:
NEW_CAPACITY_CONSTRAINT_UP(n,t,y_)$(
inv_tec(t) AND map_tec(n,t,y_) AND is_dynamic_new_capacity_up(n,t,y_)
)..
CAP_NEW(n,t,y_)
=L=
# 'Hard' constraint value
SUM(
y_prev$seq_period(y_prev, y_),
(
# New capacity in previous model period
CAP_NEW(n,t,y_prev)$(model_horizon(y_prev) AND map_tec(n,t,y_prev))
# New capacity in previous historical period
+ historical_new_capacity(n,t,y_prev)
# Otherwise, maximum initial value
# FIXME Do not use this if any of the above are non-zero
+ initial_new_capacity_up(n,t,y_)
)
* k_gncu(n,t,y_prev,y_)
)
# 'Soft' relaxation of constraint
+ (
CAP_NEW_UP(n,t,y_) * (POWER(1 + soft_new_capacity_up(n,t,y_), duration_period(y_)) - 1)
)$soft_new_capacity_up(n,t,y_)
# Additional relaxation for calibration and debugging
%SLACK_CAP_NEW_DYNAMIC_UP% + SLACK_CAP_NEW_DYNAMIC_UP(n,t,y_)
;
This feels much easier to read, especially when also looking at our Python code that uses the Black code style.
If we were to formalize such a style, it might include rules like the following:
- Operators must appear either:
- Inline, with spaces on either side, or,
- At the start of a new line, followed by the operand or opening parenthesis.
- Parentheses:
- Parentheses must not appear around single identifiers, including in GAMS dollar conditionals (
a$b(x,y,z), not(a)$(b(x,y,z)). - Closing parentheses must appear either:
- On the same line as the corresponding opening parenthesis, if the parenthesized expression fits within the line length, or else
- On a line with the same indentation as the line containing the corresponding opening parenthesis.
- Parentheses must not appear around single identifiers, including in GAMS dollar conditionals (
- Comments:
- Inline comments in expressions must use the
#character and must match the local indentation.
- Inline comments in expressions must use the
- Blank lines within expressions (parameter assignments, equations, etc.) should be used to clarify structure.
- Short aliases for set names should be used.
- Indent using 2 spaces.
- Wrap lines at 120 characters.
Further thoughts:
- As with 'black', the advantage would be "[f]ormatting [that] becomes transparent after a while [so that] you can focus on the content instead."
- Like semantic line breaks in Mention Semantic Line Breaks in the code style #927/Add semantic line breaks to code style docs #931, this should be applied incrementally as parts of the GAMS code are added/revised. I don't think we need a massive PR to rewrite all the code.
- I am not aware of any auto-formatter for GAMS code similar to
rufforblack. If one exists, we could consider to use it. If not, then I think it is probably excessive effort to create one, especially given Migrating away from GAMS #879.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
discussDiscussion, design, and planningDiscussion, design, and planningenhNew features & functionalityNew features & functionalitygamsRelated to the GAMS core formulationRelated to the GAMS core formulationhelp wanted