Zz/cyme improvements by tarekelgindy · Pull Request #54 · NLR-Distribution-Suite/ditto

tarekelgindy · 2026-01-06T00:54:09Z

No description provided.

…cyme reader

…ch objects in CYME

…sed since spacings not provided in CYME

…quipment itself

…ting zero length lines to be small instead

…s in different versions of cyme

…fferent versions of cyme

…of cyme

…alidation in gdm first...

Copilot

Pull request overview

This PR adds comprehensive CYME reader functionality and improves the OpenDSS writer with name sanitization and bug fixes. The changes enable reading CYME distribution network files and converting them to the internal data model format.

Key Changes:

New CYME reader implementation with component and equipment mappers for buses, transformers, loads, capacitors, switches, fuses, reclosers, and branches
OpenDSS writer improvements including name sanitization (replacing spaces and dots with underscores) and attribute name corrections (nominal_voltage → rated_voltage)
Quantity type updates in OpenDSS reader (Positive* types → regular types for broader compatibility)

Reviewed changes

Copilot reviewed 58 out of 59 changed files in this pull request and generated 48 comments.

Show a summary per file

File	Description
tests/test_cyme/test_cyme_reader.py	New test file for CYME reader functionality
tests/test_opendss/test_opendss_reader.py	Updated export path structure and directory creation
src/ditto/readers/cyme/reader.py	Main CYME reader with component parsing and voltage assignment logic
src/ditto/readers/cyme/utils.py	Utility functions for reading CYME data files
src/ditto/readers/cyme/components/*.py	Component mappers for buses, loads, transformers, branches, capacitors, switches, fuses, reclosers
src/ditto/readers/cyme/equipment/*.py	Equipment mappers for conductors, cables, transformers, protective devices
src/ditto/writers/opendss/write.py	Updated imports and voltage attribute names
src/ditto/writers/opendss/components/*.py	Added name sanitization and rated_voltage usage
src/ditto/writers/opendss/equipment/*.py	Added name sanitization and validation guards
src/ditto/readers/opendss/components/*.py	Changed Positive* quantity types to regular types
src/ditto/enumerations.py	Added RECLOSER file types to OpenDSSFileTypes enum
.gitignore	Added .DS_Store exclusion

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-06T00:58:48Z

src/ditto/writers/opendss/opendss_mapper.py

+            try:
+                mapping_function = getattr(self, "map_" + field)
+                mapping_function()
+            except Exception as e: 
+                print(f"{self.model.label} - {field}")


The try-except block catches all exceptions without any logging or specific exception handling. This can hide bugs and make debugging difficult. Consider catching specific exceptions or at least logging the error.

Copilot · 2026-01-06T00:58:48Z

src/ditto/writers/opendss/equipment/concentric_cable_equipment.py

+
+    def map_conductor_diameter(self):
+        radius = self.model.conductor_diameter.magnitude / 2
+        if radius <=0:


The comparison operators should have spaces around them. 'radius <=0' should be 'radius <= 0' for consistency with Python PEP 8 style guidelines.

src/ditto/readers/cyme/equipment/distribution_transformer_equipment.py

Copilot · 2026-01-06T00:58:49Z

src/ditto/writers/opendss/opendss_mapper.py

+                mapping_function = getattr(self, "map_" + field)
+                mapping_function()
+            except Exception as e: 
+                print(f"{self.model.label} - {field}")


Using print statements instead of logger for error messages is inconsistent with the rest of the codebase which uses loguru's logger. This should use logger.error() or logger.warning() instead.

src/ditto/readers/cyme/reader.py

Copilot · 2026-01-06T00:58:59Z

src/ditto/readers/cyme/components/distribution_load.py

+        from_bus = None
+        try:
+            from_bus = self.system.get_component(component_type=DistributionBus,name=from_bus_name)
+        except Exception as e:    


'except' clause does nothing but pass and there is no explanatory comment.

src/ditto/readers/cyme/components/distribution_load.py

src/ditto/readers/cyme/components/distribution_capacitor.py

src/ditto/readers/cyme/components/distribution_load.py

Copilot

Pull request overview

Copilot reviewed 58 out of 59 changed files in this pull request and generated 17 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-16T15:52:21Z

src/ditto/writers/opendss/components/distribution_capacitor.py

        # TODO: We're not building equipment for the Capacitors. This means that there's no guarantee that we're addressing all of the attributes in the equipment in a structured way like we are for the component.
+
+    def map_in_service(self):
+        self.opendss_dict["Enabled"] = "Yes" if self.model.in_serivce else "No"


Corrected spelling of 'in_serivce' to 'in_service'.

Copilot · 2026-01-16T15:52:21Z

src/ditto/writers/opendss/write.py

        separate_substations: bool = True,
        separate_feeders: bool = True,
    ):
+        output_folder = output_path


Variable output_folder is assigned but then reassigned on line 90 without being used. This initial assignment appears redundant and should be removed.

src/ditto/readers/cyme/components/distribution_load.py

src/ditto/readers/cyme/equipment/geometry_branch_equipment.py

src/ditto/readers/cyme/components/distribution_transformer.py

src/ditto/readers/cyme/reader.py

src/ditto/readers/cyme/utils.py

tarekelgindy

looots of comments.
Many are for clarity but a good number of changes should probably be made.

tarekelgindy · 2026-02-11T20:57:04Z

tests/test_cyme/test_cyme_reader.py

+    if not export_path.exists():
+        export_path.mkdir(parents=True, exist_ok=True)
+
+    reader = Reader(cyme_folder / cyme_network_name, cyme_folder / cyme_equipment_name, cyme_folder / cyme_load_name, '1')


Let's have a value for load_model_id ('1') here, instead of providing it directly, since some models might use 0

@tarekelgindy I've tried to address but not exactly sure what this means.

tarekelgindy · 2026-02-11T21:12:56Z

src/ditto/writers/opendss/opendss_mapper.py

-
-            return profile_name
+            try:
+                mapping_function = getattr(self, "map_" + field)


Agree with copilot that this should probably be outside of a try-except so that errors are thrown

tarekelgindy · 2026-02-11T21:17:43Z

src/ditto/writers/opendss/equipment/sequence_impedance_branch_equipment.py


    def map_name(self):
-        self.opendss_dict["Name"] = self.model.name
+        self.opendss_dict["Name"] = self.model.name.replace(" ", "_").replace(".", "_")


I believe that this might already be addressed in the existing PR #55 ?
If we merge that first, this can be removed.

tarekelgindy · 2026-02-11T21:22:30Z

src/ditto/writers/opendss/equipment/matrix_impedance_recloser_equipment.py

+
+    altdss_name = "LineCode_ZMatrixCMatrix"
+    altdss_composition_name = "LineCode"
+    opendss_file = OpenDSSFileTypes.RECLOSER_CODES_FILE.value


Some differences here between this and PR #55 that will cause a conflict.
There, line 13 is:
opendss_file = OpenDSSFileTypes.SWITCH_CODES_FILE.value
also, model and system are also provided to init function.

tarekelgindy · 2026-02-11T21:23:04Z

src/ditto/writers/opendss/equipment/matrix_impedance_branch_equipment.py


    def map_name(self):
-        self.opendss_dict["Name"] = self.model.name
+        self.opendss_dict["Name"] = self.model.name.replace(" ", "_").replace(".", "_")


This should be removed since handled in PR #55

tarekelgindy · 2026-02-19T01:22:01Z

src/ditto/readers/cyme/equipment/load_equipment.py

+        return ReactivePower(kvar, 'kilovar')
+
+    # Is this included in CYME 9.* ? It was in customer class in previous cyme versions
+    def map_z_real(self, row):


Should we always assume a full impedance load?

Not sure we have another option

tarekelgindy · 2026-02-19T01:25:03Z

src/ditto/readers/cyme/reader.py

+                "MatrixImpedanceFuseEquipmentMapper",
+            ]
+        )
+        default_conductor = BareConductorEquipment(


Should we use a rigid set of values like this?

@tarekelgindy We can remove this I think. I've never used it. Have you?

tarekelgindy · 2026-02-19T01:35:30Z

src/ditto/readers/cyme/reader.py

+            components = self.system.get_components(component_type)
+            self._add_components(components)
+
+        self._validate_model()


This seems like a sensible place to fill missing values using GDM MDC rather than having them be done in-line for many cases.

tarekelgindy · 2026-02-19T01:39:46Z

src/ditto/readers/cyme/reader.py

+
+        for vsource in voltage_sources:
+            vsource.bus.rated_voltage = (
+                vsource.equipment.sources[0].voltage * 1.732


voltage source values are assumed line-ground.
Does this mean that when setting bus voltages we're assuming line-line?
We should have a way to enforce our choice rather than assuming

We're not assuming L-L. It's just being left in L-N unless we have multiple phases in which can it's L-L

tarekelgindy · 2026-02-19T01:41:36Z

src/ditto/readers/cyme/reader.py

+                    if component_type == "DistributionTransformer":
+                        for i, winding in enumerate(obj.equipment.windings):
+                            voltage = winding.rated_voltage
+                            voltage_type = winding.voltage_type


Actually... is this where voltage type checking is being done?

Yes, this function, (which has since been updated slightly) is responsible for bus voltage assignment and wrangles most of this.

tarekelgindy

looots of comments.
Many are for clarity but a good number of changes should probably be made.

zzink-nrel · 2026-02-23T16:35:12Z

src/ditto/readers/cyme/reader.py

+
+                if mapper_name in phase_elements:
+                    phases = []
+                    for phase in ["A", "B", "C"]:


@tarekelgindy Does this create a 1ph, 2ph, and 3ph version of equipment components where the phasing isn't immediately apparent? It would be more work but I think referencing network and then section to know phasing would be better to avoid extra equipment creation.

zzink-nrel · 2026-02-23T17:00:04Z

src/ditto/readers/cyme/equipment/geometry_branch_equipment.py

+        # TODO: Understand how this is actually represented
+        return Distance(0.001,'mm')
+
+    def map_insulation_diameter(self, row, equipment_file):


@tarekelgindy Should we look to take another pass on these functions?

tarekelgindy and others added 30 commits February 26, 2025 14:01

adding initial take at bus

b3aa6b2

add distribution capacitor equipment and component

011f6d5

adding testing with data not included in repo

abdc5ca

adding general reading functionality for cyme

bb66583

moving distribution bus to components and fixing bugs

61ca60b

merging

37e8148

fixing indent problem

8e556b1

Create distribution capacitor equipment and component, add it to the …

a86dccd

…cyme reader

Implement distribution loads

4cd65e3

untested changes to read from load file not network

a2f7351

fixing breaking changes from GDM updates in opendss writer

7e257e4

fixing breaking changes from GDM updates in cyme reader

83847ce

Current chagnes for cyme reader. Needs fixes

97a400d

Removing Positive Quantities

5c931b5

complexity, byphase line comprehension, feeder, random fixes

9ac42a6

transformer byphase, feeder mapping, switches

7d45ab2

fuse and recloser additions

e04f900

feeder construction, other stuff

a74227e

neutral phasing, matrix defaults

0069b09

length unit fixes

4bc784b

adding functionality for cables to be read into matrix_impedance_bran…

6bdf1f2

…ch objects in CYME

adding functionality for concentric neutral cables to be added. Not u…

b4b38ef

…sed since spacings not provided in CYME

small update from nominal voltage to rated voltage

a3b3eac

adding concentric neutral cable writing capability in opendss

cca3dfd

cyme reader to also export to geojson and include loads

1e8b830

merging tarek and zephyr changes

70e9b98

not checking for phase mismatches for the moment...

8679948

minor fixes to cyme reader

80144d7

removing spaces in opendss outputs

5b8cf3e

opendss fixes for changing nominal voltage to rated voltage

91c8500

tarekelgindy added 13 commits November 4, 2025 19:38

ensuring unique linecodes per phase

c5e0499

fixes for missing loads

c97b84e

updating reader for unique linecodes

3095da2

removing . values from opendss outputs

576b7db

removing extra addition of equipment. This is done when parsing the e…

eaf6a7d

…quipment itself

updating names due to issues with non-uniqueness in opendss. Also set…

68eeea2

…ting zero length lines to be small instead

updates to components to fix bugs with capacitors and voltages source…

f40f192

…s in different versions of cyme

updates to equipment to fix bugs with transformers and branches in di…

5b4334f

…fferent versions of cyme

setting units for matrix impedance branch. TODO: check unit types

005a6c1

updates to equipment to fix bugs with branches in different versions …

31cf0f6

…of cyme

adding considerations for sizes which are zero. Should be caught by v…

78170a0

…alidation in gdm first...

adding a default load number

06f9302

organizing opendss data to be in a folder

f83605a

tarekelgindy requested review from AadilLatif and Copilot January 6, 2026 00:54

Copilot started reviewing on behalf of tarekelgindy January 6, 2026 00:54 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

zzink-nrel added 2 commits January 7, 2026 14:15

ability to truncate model to certain subs,feeders

ffd117e

linting, removing split phase for now

b9702ba

zzink-nrel requested a review from Copilot January 16, 2026 15:45

Copilot started reviewing on behalf of zzink-nrel January 16, 2026 15:46 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

zzink-nrel added 2 commits January 16, 2026 14:41

bug fix

9e5a8db

aligning values with GDM definitions

708a0c0

tarekelgindy commented Feb 19, 2026

View reviewed changes

read and assign voltage complexity

6a79684

zzink-nrel reviewed Feb 23, 2026

View reviewed changes

ruff and comment fixes for CYME Reader

8e3489c

Comments

Conversation

tarekelgindy commented Jan 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tarekelgindy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!