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.

Sign up for GitHub

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

Jump to bottom

Ampl export : extended exporter #2604

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

Ampl export : extended exporter #2604

wants to merge 85 commits into from

Conversation

nicolas-pierr
Copy link
Contributor

@nicolas-pierr nicolas-pierr commented Jun 7, 2023 •
edited by p-arvy
Loading

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce?
Feature :
Leverage #2597 to add new columns to the export:

  • A slack bus boolean in the bus table
  • r, g and b in Tap tables like it is already done for x
  • The regulating bus id for generators and Static var compensators that are in voltage regulation mode

This PR can't be accepted before #2597 is merged !

Nicolas Pierre added 8 commits June 2, 2023 15:19
add constructor with list instead of varargs
9379bef 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
WIP add ampl export version
dbf18e0 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
refactor all writing to BasicAmplExporter
4143d1a 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
match setVersion to return this
84987ea 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
AmplExportVersion is now an interface
54fa975 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
missing author and licence
8be12fe 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
typo in writeThreeWindingsTranformersMiddleBusesColumnsToFormatter
595b20b 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
integrating #2596 into this PR
57cd659 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
@nicolas-pierr nicolas-pierr self-assigned this Jun 7, 2023
Nicolas Pierre added 7 commits June 7, 2023 17:10
fix function accepting only implementation and not interface
73884d9 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
Accept version in parameters of AmplExport service
8c67231 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
add header file indicating version
fc3adf0 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
fix test because of new version parameter
38eba0a 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
Merge remote-tracking branch 'origin/main' into ampl/export-slack
639bbd5
fix BasicAmplExporter according to b7f9c3d
a0160f3 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
clean
261c0e0 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
@nicolas-pierr nicolas-pierr requested review from geofjamg and Hadrien-Godard and removed request for annetill June 13, 2023 12:44
Nicolas Pierre added 7 commits June 13, 2023 14:50
add networkUtil
0770a2c 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
add getters for inheritance
54dd6c1 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
added ExtendedAmplExporter
7e559a9 
remove static getFactory

Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
added new columns :
353c1f1 
Slack bus boolean in Bus
r g and b in TapSteps
regulating bus id in Generators and Svc

Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
wip tests
bde0378 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
export is in busView
6e5cb2d 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
WIP tap tests
334e7e0 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
clean public for tests
3e11f7b 
Signed-off-by: Nicolas Pierre <nicolas.pierre@artelys.com>
@sonarcloud
Copy link

sonarcloud bot commented Jun 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

91.9% 91.9% Coverage
0.3% 0.3% Duplication

So-Fras
So-Fras requested changes Sep 5, 2024
View reviewed changes
Copy link
Member

@So-Fras So-Fras left a comment

Choose a reason for hiding this comment

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

Thanks for the work, I left some remarks.
The duplication in ExtendedAmplExporterV1 is a real pain but I am not sure that it is possible to write a cell at a specific index with the current TableFormatter. I am going to ask for more opinions on that.

ampl-converter/pom.xml Show resolved Hide resolved
ampl-converter/src/main/java/com/powsybl/ampl/converter/version/ExtendedAmplExporterV1.java Outdated Show resolved Hide resolved
ampl-converter/src/test/java/com/powsybl/ampl/converter/AmplNetworkWriterTest.java Outdated Show resolved Hide resolved
p-arvy and others added 6 commits September 11, 2024 14:42
@p-arvy
Update version of iidm extensions
c6fc222 
Signed-off-by: p-arvy <pierre.arvy@artelys.com>
@p-arvy
Remove properties override in TUs
1e643c1 
Signed-off-by: p-arvy <pierre.arvy@artelys.com>
@p-arvy
Merge branch 'refs/heads/main' into ampl/extended-exporter
0327b6a
@rolnico
initialize TableFormatterHelper
661e9c1 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
finish TableFormatterHelper
6e12d01 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@rolnico
use record to lower number of method parameters
aafe76a 
Signed-off-by: Nicolas Rol <nicolas.rol@rte-france.com>
@So-Fras So-Fras assigned p-arvy and unassigned pjeanmarie and nicolas-pierr Sep 17, 2024
@So-Fras
Copy link
Member

So-Fras commented Sep 18, 2024

@rolnico worked on the duplication topic and suggests 2 solutions in 2 different branches: nro/ampl-extended-exporter and nro/ampl-extended-exporter-with-position. @olperr1 or @flo-dup, could you have a look at them? Thanks a lot!

@rolnico
Copy link
Member

rolnico commented Sep 18, 2024

@rolnico worked on the duplication topic and suggests 2 solutions in 2 different branches: nro/ampl-extended-exporter and nro/ampl-extended-exporter-with-position. @olperr1 or @flo-dup, could you have a look at them? Thanks a lot!

(Personally I find the nro/ampl-extended-exporter-with-position branch solution better)

@olperr1
Copy link
Member

olperr1 commented Sep 19, 2024

@rolnico worked on the duplication topic and suggests 2 solutions in 2 different branches: nro/ampl-extended-exporter and nro/ampl-extended-exporter-with-position. @olperr1 or @flo-dup, could you have a look at them? Thanks a lot!

(Personally I find the nro/ampl-extended-exporter-with-position branch solution better)

The nro/ampl-extended-exporter-with-position branch solution is great. Nice work!

I pushed an additional commit in it to use index constants instead of their numeric values (to be more robust and increase readability), but I think it can be merged in this PR.

Note that #3120 can easily be adapted to use the TableFormatterHelper introduced by this solution.

So-Fras and others added 2 commits September 20, 2024 16:46
@p-arvy
Rename ExtendedAmplExporter
7d60ce7 
Signed-off-by: p-arvy <pierre.arvy@artelys.com>
@p-arvy
Modify extended ampl exporter javadoc
6378899 
Signed-off-by: p-arvy <pierre.arvy@artelys.com>
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Sep 20, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
86.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@So-Fras
Copy link
Member

So-Fras commented Sep 22, 2024

I am sorry @p-arvy , I forgot to ask for the documentation... I put the "ready to be merged" label a bit too early.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annetill annetill annetill left review comments

@pjeanmarie pjeanmarie pjeanmarie left review comments

@p-arvy p-arvy p-arvy left review comments

@olperr1 olperr1 olperr1 requested changes

@Hadrien-Godard Hadrien-Godard Hadrien-Godard approved these changes

@So-Fras So-Fras So-Fras approved these changes

@geofjamg geofjamg Awaiting requested review from geofjamg

Requested changes must be addressed to merge this pull request.

Assignees

@p-arvy p-arvy

Labels
October 2024 release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@nicolas-pierr @So-Fras @rolnico @olperr1 @annetill @Hadrien-Godard @pjeanmarie @p-arvy