Skip to content
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

To/#28 node subnet mapper #49

Merged
merged 58 commits into from
May 31, 2021
Merged

To/#28 node subnet mapper #49

merged 58 commits into from
May 31, 2021

Conversation

t-ober
Copy link
Contributor

@t-ober t-ober commented May 18, 2021

Resolves #28

Implemented a GridGraphBuilder that builds the graph for the grid and thereby identifies the diferent subgrids.

Note for the GridPreparator:
I could have included filtering in the GridGraphBuilder but I originally thought I would have other things to filter out and still think there will be other things that come up that I will have to add there

@t-ober t-ober marked this pull request as ready for review May 20, 2021 15:58
Copy link
Member

@ckittl ckittl left a comment

Choose a reason for hiding this comment

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

First partial review, will have to continue.

build.gradle Outdated
@@ -50,6 +50,18 @@ dependencies {
// https://mvnrepository.com/artifact/org.scalameta/scalafmt-dynamic
implementation group: 'org.scalameta', name: "scalafmt-dynamic_${scalaVersion}", version: '2.7.5'


/* FIXME: Rely on stable version, when available */
implementation('com.github.ie3-institute:PowerSystemDataModel:2.0-SNAPSHOT') {
Copy link
Member

Choose a reason for hiding this comment

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

This issue will be resolved later. Please adapt build.gradleto point to version 1.5.3 for PowerSystemUtils (publication on maven central pending) and version 2.0.1 for PowerSystemDataModel, when ie3-institute/PowerSystemDataModel#362 is merged and artifact is published as well.

src/main/python/powerFactory2json/pf2json.py Outdated Show resolved Hide resolved
element["conElms"] = []
for con_elm in raw_element.GetConnectedElements():
element["conElms"].append(get_attribute_dict(con_elm, attributes4export["conElms"], True))

# export ids of nodes the edges are connected to
if (raw_element.GetClassName() in pf_edges):
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason, you treat lines and switches differently, here? I see, the bus information are included in "conElms", but why don't you choose a similar way or at least naming here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tbh I went for the conElms approach because i missed that i can get the ids via edge.bus1 more directly. This way is much more convenient since I don't have to perform a dozen checks for whether there are exactly two connected elements and whether these are configured correctly which made the code much more elaborate and harder to understand. I then excluded the exporting of conElms lines and switches since I don't need the parameter anymore but I still do need the parameter for nodes as there is no more convenient way (for checking if there is an external net at the node). I will probably scratch the parameter for transformers as welll.

Regarding naming I sticked relatively closely to the pf variable naming as this is the way we name all other variables. Also conElms was the connected element as an object and with bus[12]Id I only export the id of the element.

@@ -50,8 +50,19 @@ dependencies {
// https://mvnrepository.com/artifact/org.scalameta/scalafmt-dynamic
implementation group: 'org.scalameta', name: "scalafmt-dynamic_${scalaVersion}", version: '2.7.5'


implementation('com.github.ie3-institute:PowerSystemDataModel:2.0.0') {
Copy link

Choose a reason for hiding this comment

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

Severe OSS Vulnerability: ### pkg:maven/junit/[email protected]
1 vulnerability has been found in transitive dependency of pkg:maven/com.github.ie3-institute/[email protected]


[CVE-2020-15250] In JUnit4 from version 4.7 and before 4.13.1, the test rule TemporaryFolder cont...

In JUnit4 from version 4.7 and before 4.13.1, the test rule TemporaryFolder contains a local information disclosure vulnerability. On Unix like systems, the system's temporary directory is shared between all users on that system. Because of this, when files and directories are written into this directory they are, by default, readable by other users on that same system. This vulnerability does not allow other users to overwrite the contents of these directories or files. This is purely an information disclosure vulnerability. This vulnerability impacts you if the JUnit tests write sensitive information, like API keys or passwords, into the temporary folder, and the JUnit tests execute in an environment where the OS has other untrusted users. Because certain JDK file system APIs were only added in JDK 1.7, this this fix is dependent upon the version of the JDK you are using. For Java 1.7 and higher users: this vulnerability is fixed in 4.13.1. For Java 1.6 and lower users: no patch is available, you must use the workaround below. If you are unable to patch, or are stuck running on Java 1.6, specifying the java.io.tmpdir system environment variable to a directory that is exclusively owned by the executing user will fix this vulnerability. For more information, including an example of vulnerable code, see the referenced GitHub Security Advisory.

CVSS Score: 5.5

CVSS Vector: CVSS:3.0/AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:N/A:N


(at-me in a reply with help or ignore)

@t-ober t-ober requested a review from ckittl May 28, 2021 07:51
@ckittl ckittl merged commit fc38692 into main May 31, 2021
@ckittl ckittl deleted the to/#28-NodeSubnetMapper branch May 31, 2021 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement a NodeSubnetMapper
3 participants