Refactor PFDL code #60

maxhoerstr · 2024-08-20T11:34:06Z

Make code more modular to enable the reuse of the code (e.g.. make more functions and methods single purpose)
Fix various bugs in the petri net generation
Renaming
Format code and add docstrings
Allow a PFDL program to be passed over to the scheduler as a string (currently, only a file path was allowed)
Add various quality of live features/fixes
Allow the free configuration of the "start task" (former productionTask)
Allow integers as loop limits and negative numbers

- Create a tree where each node contains a group id, to map the call hierarchy of the different tasks

There were still parent group ids used. They are unnecessary now, since the structure of the calls is realised with a tree structure (Node)

- Petri Net images and dot files are no always named "petri_net" - Append Call Tree from PetriNetGenerator to dot file for extension

- Cluster individual components of the PFDL (Service, Tasks, ..) - Save this information in the dot file

- Fix bug in drawing of petri net for extension - Move creation of folders on better suiting location - Remove unnecessary `used_in_extension` attributes and params

- Fix typos and remove comments - Temporarily disable removal of nodes from the petri net - Publish PFDL string to dashboard - Rewrite methods so PFDL string can be extracted from the file and used

- Send Petri Net update before Order is finished - Remove counting loop subgraph in dot file

This allows for more modularity so future plugin users can choose what to reuse if the overwrite the init function of the scheduler.

- Use the term "uuid" wherever a uuid is assigned - Avoid inconsistent variable naming

- Enable plugins to overload this method

- the changed file might have caused troubles when running it while the MFDL plugin is active

- Enable reusing parts of the method when multiple types per variable are allowed

The way that the nodes (places and transitions) in the petri net were clustered with the help of SNAKES was wrong. Now, the cluster is not overwritten anymore, but correctly added in the intended way. This resolves a bug where the deletion of specific nodes resulted in an KeyError in the snakes library.

- Adjust docstrings - Remove unused attribute 'cluster'

- Add definition for variable type - Apply changes to tree visitor - Adjust tests

- Instead of limiting the starting task name to 'productionTask', it can now be modified by passing the desired name to the Process

OliverStolzBO

Looks good overall, I only figured out some minor improvements that could be applied to increase the code quality.

OliverStolzBO · 2024-08-23T09:07:24Z

pfdl_scheduler/petri_net/drawer.py

@@ -89,6 +90,10 @@ def draw_clusters(clust, attr):
    attr["style"] = DEFAULT_CLUSTER_STYLE


+def draw_clusters(clust, attr):


What is the intention of duplicating the method above, including the same number of attributes? One version should be removed (they are actually doing the same, since DEFAULT_CLUSTER_STYLE is never reassigned). Also, consider renaming clust into _ since it is unused.

OliverStolzBO · 2024-08-23T09:08:26Z

pfdl_scheduler/petri_net/generator.py

@@ -7,6 +7,7 @@
 """Contains the PetriNetGenerator class."""

 # standard libraries
+import copy
 from typing import Any, Callable, Dict, List, OrderedDict, Tuple


unused import

Copy is used in Line 54 :)

Sorry I marked the wrong line, this comment refers to the Tuple import in the line below

OliverStolzBO · 2024-08-23T09:11:57Z

pfdl_scheduler/petri_net/generator.py

-                    )
-                    node.cluster.add_node(current_connection_id)
+                    current_connection_uuid = create_transition("connection", "", self.net, node)
+                    # node.cluster.add_child(Cluster(current_connection_uuid))


remove comment

OliverStolzBO · 2024-08-23T09:21:05Z

pfdl_scheduler/petri_net/generator.py

        )
        cluster = Cluster([parallel_loop_started])
-        node.cluster.add_child(cluster)
+        # node.cluster.add_child(cluster)


remove comment

OliverStolzBO · 2024-08-23T09:21:34Z

pfdl_scheduler/petri_net/generator.py

-            # self.net.remove_place(self.task_started_id)
-
+        if self.net.has_place(place_uuid):
+            # self.net.clusters._nodes.add(place_uuid)


remove comment

OliverStolzBO · 2024-08-23T09:41:52Z

pfdl_scheduler/utils/parsing_utils.py

@@ -93,6 +94,29 @@ def write_tokens_to_file(token_stream: CommonTokenStream) -> None:
            file.write(token_text + "\n")


+def extract_content_and_file_path(program):


program: str
return type: Tuple[str, str]

OliverStolzBO · 2024-08-23T09:48:07Z

pfdl_scheduler/scheduler.py

+            self.setup_scheduling(draw_petri_net)
+
+            # enable logging
+            # self.attach(LogEntryObserver(self.scheduler_uuid))


is this still useful?

OliverStolzBO · 2024-08-23T10:01:06Z

tests/test_files/scheduling/task_synchronisation.pfdl

@@ -20,7 +20,7 @@ Struct CuttingResult
    sheet_parts: SheetPart[]
 End

-Struct TransportOrderStep
+Struct TransportOrderStep_Struct


Maybe remove the _ in the Struct and Service names here to be consistent with the other test files

OliverStolzBO · 2024-08-23T10:01:25Z

tests/test_files/scheduling/task_synchronisation.pfdl

@@ -50,14 +50,14 @@ Task cuttingTask
 End

 Task transport
-    Transport
+    Transport_Service


See comment above

OliverStolzBO · 2024-08-23T10:09:14Z

tests/unit_test/test_pfdl_tree_visitor.py

+            array = Array("string")
+            array.length = 10


This lines are meaningless and can be removed

Incorporate feedback regarding the quality of the code (fix typos, remove comments/ unused packages, fix naming)

OliverStolzBO · 2024-08-23T13:05:41Z

tests/test_files/scheduling/task_synchronisation.pfdl

@@ -20,7 +20,7 @@ Struct CuttingResult
    sheet_parts: SheetPart[]
 End

-Struct TransportOrderStep_Struct
+Struct TransportOrderStep


Using keywords of other plugins will cause problems there, so the Struct needs to have a different name than TransportOrderStep (same for Transport)

maxhoerstr added 30 commits April 12, 2023 08:25

Do not generate callbacks if used in extension

59bd720

Save petri net as dot file in extension

6e261eb

Remove unnecessary new line character

673467f

Add group ids for places in petri net

2298d3e

Save group ids in transitions

85f43f2

Add id of parent group

5e472fd

Fix missing args in method calls

95ecbb2

Create a tree with group ids while generating

8b62e42

- Create a tree where each node contains a group id, to map the call hierarchy of the different tasks

Save petri net tree into media folder

4c33691

Remove now unused parent group ids

e74f9fe

There were still parent group ids used. They are unnecessary now, since the structure of the calls is realised with a tree structure (Node)

Save petri net tree in dot file

d8c99a0

Fix drawing of petri net

297abdd

- Petri Net images and dot files are no always named "petri_net" - Append Call Tree from PetriNetGenerator to dot file for extension

Add identifier in dot file for call tree

d4eeaf0

Remove comment in dot file

53d22e8

Enforce passing of file name for extension

24cf7b3

Fix bugs

eb8ad6c

Cluster petri net components

d3186fa

- Cluster individual components of the PFDL (Service, Tasks, ..) - Save this information in the dot file

Draw dot file every time

60a8318

Fix minor issues

f2fdb89

- Fix bug in drawing of petri net for extension - Move creation of folders on better suiting location - Remove unnecessary `used_in_extension` attributes and params

Minor changes

8106ef9

- Fix typos and remove comments - Temporarily disable removal of nodes from the petri net - Publish PFDL string to dashboard - Rewrite methods so PFDL string can be extracted from the file and used

Small fixes

91129fd

- Send Petri Net update before Order is finished - Remove counting loop subgraph in dot file

Save order specific petri net image and dot file

eb3c641

Use scheduler id for logging

9a00ebc

Remove saving of tokens

325edb7

Use correct transition list

0fdfd3a

Cast loop limit to int before using it

663c7ad

Add correct casting

eeda72e

Allow integers in counting loop limits

c126d34

Support integer loop limits in scheduler

72da9bb

Move content of init into separate functions

df88d25

This allows for more modularity so future plugin users can choose what to reuse if the overwrite the init function of the scheduler.

OliverStolzBO and others added 19 commits January 9, 2024 15:24

Rename id to uuid for consistency

3faa540

- Use the term "uuid" wherever a uuid is assigned - Avoid inconsistent variable naming

Add helper method to cast datatypes from string

8059056

Enable empty comments in PFDL programs

e48e0be

Move helper method into semantic error checker

68809a0

- Enable plugins to overload this method

Add unit test for integer loop limits

02a070c

Rename dict key for consistency

674b648

Edit test file to avoid naming conflicts

61c72ef

- the changed file might have caused troubles when running it while the MFDL plugin is active

Split method

5bd0be9

- Enable reusing parts of the method when multiple types per variable are allowed

Update generator

73f0fb8

- Adjust docstrings - Remove unused attribute 'cluster'

Extend grammar by variable type representation

61e57a9

- Add definition for variable type - Apply changes to tree visitor - Adjust tests

Fix old clustering code

a7e04a1

Allow negative numbers

ab57358

Update generated lexer file

8498513

Add better documentation for petri net logic

c737873

Enable flexible starting task names for plugins

ea838f5

- Instead of limiting the starting task name to 'productionTask', it can now be modified by passing the desired name to the Process

Merge branch 'main' into refactoring

54d5757

Update transition ids that were broken after merge

83bfdfe

Remove more code that was introduced due to the merge

0c467fc

maxhoerstr requested a review from OliverStolzBO August 20, 2024 11:34

maxhoerstr added 2 commits August 21, 2024 15:54

Add constants that were lost due to merge

9a49858

Check if there is only a single task call in parallel loop

f180401

OliverStolzBO reviewed Aug 23, 2024

View reviewed changes

Incorporate PR feedback

e6a9bca

Incorporate feedback regarding the quality of the code (fix typos, remove comments/ unused packages, fix naming)

maxhoerstr requested a review from OliverStolzBO August 23, 2024 12:53

OliverStolzBO requested changes Aug 23, 2024

View reviewed changes

Rename test program variables

b3fa710

OliverStolzBO approved these changes Aug 23, 2024

View reviewed changes

maxhoerstr merged commit 74ac2f6 into main Aug 23, 2024
6 checks passed

maxhoerstr deleted the refactoring branch August 26, 2024 09:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor PFDL code #60

Refactor PFDL code #60

maxhoerstr commented Aug 20, 2024

OliverStolzBO left a comment

OliverStolzBO Aug 23, 2024

OliverStolzBO Aug 23, 2024

maxhoerstr Aug 23, 2024

OliverStolzBO Aug 23, 2024

OliverStolzBO Aug 23, 2024

OliverStolzBO Aug 23, 2024

OliverStolzBO Aug 23, 2024

OliverStolzBO Aug 23, 2024

OliverStolzBO Aug 23, 2024

OliverStolzBO Aug 23, 2024

OliverStolzBO Aug 23, 2024

OliverStolzBO Aug 23, 2024

OliverStolzBO Aug 23, 2024

		@@ -89,6 +90,10 @@ def draw_clusters(clust, attr):
		attr["style"] = DEFAULT_CLUSTER_STYLE


		def draw_clusters(clust, attr):

		@@ -93,6 +94,29 @@ def write_tokens_to_file(token_stream: CommonTokenStream) -> None:
		file.write(token_text + "\n")


		def extract_content_and_file_path(program):

Refactor PFDL code #60

Refactor PFDL code #60

Conversation

maxhoerstr commented Aug 20, 2024

OliverStolzBO left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment