Skip to content

Documentation: add advanced programming terminology documentation #1006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mgovers
Copy link
Member

@mgovers mgovers commented May 30, 2025

Prep for design discussion on main model cleanup as well as follow-up of discussions during the meet-up.

@mgovers mgovers requested a review from Copilot May 30, 2025 14:41
@mgovers mgovers self-assigned this May 30, 2025
@mgovers mgovers added the documentation Improvements or additions to documentation label May 30, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the documentation by updating terminology in the components description and adding an extensive glossary of advanced programming terminology.

  • Updated the components documentation to change the electrical parameter description.
  • Added a new documentation file for terminology and updated the index for easy navigation.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
docs/user_manual/components.md Updated the description of the “link” attribute’s parameter.
docs/index.md Added a reference to the advanced terminology documentation.
docs/advanced_documentation/terminology.md Added a comprehensive glossary covering programming terminology.
Comments suppressed due to low confidence (1)

docs/advanced_documentation/terminology.md:202

  • Typo found: 'industy' should be corrected to 'industry'.
collaboration between ICT for industy and power grid model.

@mgovers mgovers marked this pull request as ready for review June 2, 2025 07:26
Copy link

sonarqubecloud bot commented Jun 2, 2025

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo changed the title documentation: add advanced programming terminology documentation Documentation: add advanced programming terminology documentation Jun 2, 2025
@mgovers mgovers enabled auto-merge June 2, 2025 12:00
@nitbharambe
Copy link
Member

Its quite nice that all possible cases are collected and that we are aware of uncertain things present in the library. But I am not sure where this page should lie as the complete list is irrelevant to the user and maybe also for a less involved opensource developer. Nevertheless, quite useful.

What if

  • We split whole of bad input + Incorrect results, Nonsensical results, Incorrect exceptions into one section and rest into other? This section can lie in the user manual.
  • Or we have a small section in user manual mentioning these different cases of issues linking to their explanation in this page.

It is the users' responsibility to provide correct input data. See also [below](#bad-input).
* If the input data provided to the power grid model is incorrect, the power grid model cannot
guarantee that it will be able to find and handle that correctly. The behavior is
[undefined](#undefined-behavior-ub-as-a-bug). This is also true for any other native interface.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mention the phrase "This is also true for any other native interface" several times, and I agree with it. However, since it is such a strong statement, can we cite this? Probably not, but worth doing a quick search.

Comment on lines +51 to +53
* The Python wrapper contains human-readable code. Some people consider this a weakness of Python as
a programming language. Always make sure to treat your Python code and configuration files as
company secrets.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though I agree with what is written here, does it belong under the security vulnerability heading?


See also [below](#undefined-behavior-ub-as-a-bug).

Although the power grid model takes all the care they can in preventing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Although the power grid model takes all the care they can in preventing
Although the power grid model takes all the care it can in preventing

Comment on lines +87 to +88
Sometimes, exceptions are raised when they should not be. For example, a `NotObservableError` that
is raised, even though the scenario actually contains an observable grid, would be a bug. They can
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the wording a bit weird here. Maybe something along the lines of:

Suggested change
Sometimes, exceptions are raised when they should not be. For example, a `NotObservableError` that
is raised, even though the scenario actually contains an observable grid, would be a bug. They can
Sometimes, exceptions are raised when they should not be. For example, a `NotObservableError` that
is raised when the current scenario contains an observable grid, would be a bug. They can

`u_rated` on a node is not allowed. Unsupported input will always be documented in the
power grid model documentation.

Checking every single combination of input parameters would come at a significant performance cost
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Checking every single combination of input parameters would come at a significant performance cost
Checking every single combination of input parameters would come at a significant performance cost.

In non-spurious edge cases, the current behavior of the power grid model is actually correct.

Many edge cases are inherent to the problem statement, like the amount of iterations it takes before
a solution is found in an iterative calculation method, are not problematic. Even if you encounter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What did you intend to say with "are not problematic"? Phrasing seems weird.


If you encounter any edge case, please double-check your data quality, e.g., by running our
[data validator](../user_manual/data-validator.md) and verifying that the source of your data is
up-to-date and correct. Please also try things like increasing the error tolerance to see if that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
up-to-date and correct. Please also try things like increasing the error tolerance to see if that
up-to-date and correct. Please also try things like increasing the error tolerance or the number of iterations (when applicable) to see if that


##### Undefined behavior (subclass)

Undefined behavior is also a subclass of undefined behavior that is defined as follows:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this phrasing intended? UB is a subclass of UB itself?

used in test and development environments to prevent internal bugs.

To prevent, find and fix harder-to-find instances of undefined behavior, the power grid model uses
tools like AddressSanitizer and Sonar Qube Cloud. We strongly recommend all users to do the same -
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: I believe this is the correct naming.

Suggested change
tools like AddressSanitizer and Sonar Qube Cloud. We strongly recommend all users to do the same -
tools like AddressSanitizer and Sonar QubeCloud. We strongly recommend all users to do the same -

especially the ones using the C API.

If you find or expect any undefined behavior, please report it and/or
[contribute](../user_manual/model-validation.md#test-case-creation) the repro case, as doing so
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[contribute](../user_manual/model-validation.md#test-case-creation) the repro case, as doing so
[contribute](../user_manual/model-validation.md#test-case-creation) with a reproducible case, as doing so

@figueroa1395
Copy link
Contributor

Its quite nice that all possible cases are collected and that we are aware of uncertain things present in the library. But I am not sure where this page should lie as the complete list is irrelevant to the user and maybe also for a less involved opensource developer. Nevertheless, quite useful.

What if

  • We split whole of bad input + Incorrect results, Nonsensical results, Incorrect exceptions into one section and rest into other? This section can lie in the user manual.
  • Or we have a small section in user manual mentioning these different cases of issues linking to their explanation in this page.

I think that as long as we explicitly say that the list is not exhaustive and that this belongs in the advanced documentation, it should be okay to keep here and public. However, this would function sort of a public guideline on how do we tackle and consider edge cases, UB, etc., which is very important. Hence it would be nice if the whole team can take a look and see whether we agree on everything written, or if some things need to be added, changed or removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants