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

Feature/gql2 566: PGM to GAIA conversion #282

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

minhquan-hcmut
Copy link

@minhquan-hcmut minhquan-hcmut commented Nov 11, 2024

Idea for Gaia conversion, approach and modelling

@nitbharambe nitbharambe added the do-not-merge This should not be merged label Nov 11, 2024
@nitbharambe nitbharambe marked this pull request as draft November 11, 2024 13:33
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the legacy Vision leftover from source

Copy link
Member

Choose a reason for hiding this comment

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

including updating the docstring

Comment on lines +35 to +41
def __init__(self, source_file: Optional[Union[Path, str]] = None, language: str = "en"):
mapping_file = Path(str(DEFAULT_MAPPING_FILE).format(language=language))
if not mapping_file.exists():
raise FileNotFoundError(f"No Vision Excel mapping available for language '{language}'")
self._id_reference: Optional[IdReferenceFields] = None
source = VisionExcelFileStore(file_path=Path(source_file)) if source_file else None
super().__init__(mapping_file=mapping_file, source=source)
Copy link
Contributor

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo Nov 11, 2024

Choose a reason for hiding this comment

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

Please unify with other excel converters and make mapping_file a parameter

Copy link
Member

@mgovers mgovers left a comment

Choose a reason for hiding this comment

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

a test case might be needed for this.

if len(attr_data.columns) == 1:
pgm_data[attr] = attr_data.iloc[:, 0]
elif component == 'asym_load':
pgm_data[attr] = attr_data.iloc[:, 0:3]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pgm_data[attr] = attr_data.iloc[:, 0:3]
pgm_data[attr] = attr_data.iloc[:, :3]

Comment on lines +38 to +44
p_nom: float,
wind_speed: float,
cut_in_wind_speed: float = 3.0,
nominal_wind_speed: float = 14.0,
cutting_out_wind_speed: float = 25.0,
cut_out_wind_speed: float = 30.0,
axis_height: float = 30.0,
Copy link
Member

Choose a reason for hiding this comment

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

why reformat?

Copy link
Member

Choose a reason for hiding this comment

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

including updating the docstring

Comment on lines +41 to +49
# r0:
# power_grid_model_io.functions.both_zeros_to_nan:
# value: R0
# other_value: X0
# x0:
# power_grid_model_io.functions.both_zeros_to_nan:
# value: X0
# other_value: R0
# c0: C0
Copy link
Member

Choose a reason for hiding this comment

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

should be removed, right?

Copy link
Member

Choose a reason for hiding this comment

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

this file contains quite a significant amount of commented out lines. should those be removed?

@nitbharambe
Copy link
Member

Relates to #5

@minhquan-hcmut
Copy link
Author

a test case might be needed for this.

Files in the below folder are used to test the io.
https://github.com/Alliander/dnr-sandbox/pull/46

@mgovers
Copy link
Member

mgovers commented Nov 11, 2024

a test case might be needed for this.

Files in the below folder are used to test the io. Alliander/dnr-sandbox#46

that's Alliander-internal data that cannot be shared outside (good thing it's not visible to people who don't have access to Alliander). Needs to be anonymized, randomized and reorganized to something that can be shared with other people.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge This should not be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants