You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This issue is to review our development and style guides to ensure that they are up to date with our newest development practices. This is to save time for both external contributors and our code reviewers, especially when there are multiple pieces of guiding documents at different places to keep track of.
which are described in the `user guide <https://simulationresearch.lbl.gov/modelica/releases/v10.0.0/help/Buildings_Fluid_Interfaces_UsersGuide.html#Buildings.Fluid.Interfaces.UsersGuide>`_ of this package.
Otherwise, it becomes difficult to ensure that the implementation is numerically robust.
This has not been enforced, especially when we want (a) port placements on the icon to be more convenient in the specific application model or (b) more descriptive port names, such as port_hotWatIn instead of port_a.
#. In the source code, the instances must be ordered as follows:
- First, list `Boolean` parameters,, then `Integer` parameters and then `Real` parameters.
- Next, list inputs, then outputs, followed by blocks.
- Protected instances are below all the public instances and follow the same instance ordering rules.
- Within the above order, list scalar values before arrays,
but prioritize groupings based on model specific similarities.
The parameter-variable-input-output declaration order has been enforced outside of the OBC package as well. I suggest elevating this item be applicable to the library in general.
The developer that introduces a new model, block or a function must:
1. Implement at least one example or validation model that serves as a unit test for each model, block and function,
and run the unit tests.
Unit tests should cover all branches of ``if-then`` constructs and
all realistic operating modes of the system represented by the model.
2. In the `info` section of the validation model, describe to others the intent of the unit test.
For example, an air handler unit controller test could describe
"This model verifies that as the cooling load of the room increases, the controller
first increases the mass flow rate setpoint and then reduces the supply temperature setpoint."
(a). We often emphasise that the model should be able to get in and out of zero flow rate without problem. This should be added to the guideline.
(b). As discussed in IBPSA working group meetings, "validation" may lead to people thinking that we tested the models against experimental data, but that's not our case. We should clarify this.
The wiki is not part of the code in this repo, but I think it is still suitable to have the discussion here.
Now I have pasted three different links regarding our coding guidelines that developers and contributors need to keep track of. As the wiki presents a more concise workflow, we should include links of relevant documents there so that authors could find them easily. "Review development guidelines" should be part of the workflow to start with.
Item (5) "When done with the implementation" - We should add that the links in the documentation should be tested. Broken links have been a problem we have frequently needed to circle back to.
Item (5)(iii) "run the unit tests" - A link to the unit test wiki should be added here. In addition, we should be more specific than To avoid introducing bugs in the library, do not commit changes that you cannot explain. For example:
i. Small changes due to numerical noise and additions of nonlinear systems of size 1 can be directly accepted.
ii. More significant changes must be understood and deemed reasonable to ensure that they are intended.
The text was updated successfully, but these errors were encountered:
For reference, there are also a set of naming conventions specifically for the Templates package: https://lbl-srg.github.io/modelica-buildings-templates/more/nomenclature
Using stricter naming conventions in this context was motivated by the fact that control point connections do not have graphical annotations in templates. With those conventions, a sensor for supply air temperature is named TAirSup and its output is connected to the control input point with connect(TAirSup.y, bus.TAirSup).
This issue is to review our development and style guides to ensure that they are up to date with our newest development practices. This is to save time for both external contributors and our code reviewers, especially when there are multiple pieces of guiding documents at different places to keep track of.
In the development guide:
Thermofluid components:
modelica-buildings/Buildings/Resources/Documentation/userGuide/source/development.rst
Lines 31 to 34 in 140a250
This has not been enforced, especially when we want (a) port placements on the icon to be more convenient in the specific application model or (b) more descriptive port names, such as
port_hotWatIn
instead ofport_a
.Copying code:
modelica-buildings/Buildings/Resources/Documentation/userGuide/source/development.rst
Line 49 in 140a250
We broke this rule in
Fluid.Movers.BaseClasses.PartialFlowMachine
to avoid a circular parameter binding. Discussion in IBPSA#1705.Declaration order in OBC:
modelica-buildings/Buildings/Resources/Documentation/userGuide/source/development.rst
Lines 531 to 537 in 140a250
The parameter-variable-input-output declaration order has been enforced outside of the OBC package as well. I suggest elevating this item be applicable to the library in general.
Validation and example models:
modelica-buildings/Buildings/Resources/Documentation/userGuide/source/development.rst
Lines 654 to 667 in 140a250
(a). We often emphasise that the model should be able to get in and out of zero flow rate without problem. This should be added to the guideline.
(b). As discussed in IBPSA working group meetings, "validation" may lead to people thinking that we tested the models against experimental data, but that's not our case. We should clarify this.
In the naming conventions:
The three-letter convention:
modelica-buildings/Buildings/UsersGuide/Conventions.mo
Lines 46 to 47 in 140a250
There are exceptions in the library. A notable one is
Lvg
.Connector names:
modelica-buildings/Buildings/UsersGuide/Conventions.mo
Lines 140 to 147 in 140a250
At times we have encouraged the use of more descriptive names, such as
port_hotWatIn
instead of justport_a
. We should mention this.In the code change workflow wiki:
The wiki is not part of the code in this repo, but I think it is still suitable to have the discussion here.
i. Small changes due to numerical noise and additions of nonlinear systems of size 1 can be directly accepted.
ii. More significant changes must be understood and deemed reasonable to ensure that they are intended.
The text was updated successfully, but these errors were encountered: