Skip to content

parquet part 3 : parquet for legacy solver#3586

Merged
flomnes merged 141 commits into
developfrom
feature/parquet-for-legacy-solver
Jun 1, 2026
Merged

parquet part 3 : parquet for legacy solver#3586
flomnes merged 141 commits into
developfrom
feature/parquet-for-legacy-solver

Conversation

@guilpier-code

@guilpier-code guilpier-code commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

This PR is about ticket ANT-4633.

As we choose to break ticket ANT-4633 into several parts, this is part 3.
This PR intends to write simulation tables for legacy solver.

Please note : list of what was done can be found below, but some github comments were left to ease review.
Caution : many review comments made here are addressed in this PR 3657.

What was done :

  • remove the aggregation of all years' simulation tables into only 2 blocks (one per optimization), as well as printing them into 2 CSV files only. Instead, make each year print its own simulation table.
  • class OptimisationsSimulationTable was (physically) moved to where it more likely belongs (next to class SimulationTable)
  • class OptimisationsSimulationTable was very much simplified : it now only contains data (could now be simple struct) and the writing part was transferred to class LegacySimulationTablesWriter.
  • New class LegacySimulationTablesWriter now uses intensively the table writers developed on modeler side (in PR 3565), instead of keeping old code from class OptimisationsSimulationTable

What remains to be done (if item is checked, then it's done) :

  • continue simplifying class SimulationTable (especially there is code duplication between class SimulationTable [writeToBuffer(), buffer(), buffer_] and class CsvTableWriter [make_line(...), ...])
  • We removed unit tests on printing simulation tables into CSV format. They may be replaced by tests printing into files. We could even add more tests for thorough checks.
  • OptimisationsSimulationTable could be renamed into LegacySimulationTables, maybe more clear.
  • Consider removing class FileWriter. Indeed, it does little now, and calls another polymorphic writer (csv or parquet). So what FileWriter does could be done in the parent (abstract) class of the polymorphic writer.
  • Cucumber scripts : I made changes in class solver_input_handler and solver_output_handler, in the name of the simu table files name. are this files useful ? please check.
  • target parquet_writer was reintroduced by a merge from develop, it used to be named simulation-table-writers, much better (more accurate). Let's rename this target back.
  • (maybe fixed with a fresh clone) in debug (at least on Win), zlibd.lib not copied into build folder at configuration step. And dll like parquet, arrows + dependencies are not copied next to .exe needing them (even if they exist somewhere in the build folder after cmake config step).
  • what else ?

JasonMarechal25 and others added 30 commits October 6, 2025 11:29
…ature/parquet-new-start-after-long-pause

# Conflicts:
#	src/io/outputs/CMakeLists.txt
When vcpkg is used (e.g. for Arrow/Parquet), it prepends its paths to
CMAKE_PREFIX_PATH. This causes find_dependency(absl) inside
ortoolsConfig.cmake to resolve vcpkg's absl (lts_20240722) instead of
the version OR-Tools was compiled against (lts_20250512), leading to
thousands of linker errors due to ABI incompatibility.

Fix: before find_package(ortools), detect the OR-Tools install prefix
and set absl_DIR/re2_DIR to the co-installed versions. Supports both
lib/ (Ubuntu, Windows) and lib64/ (Oracle Linux 8) layouts.
Apply the same absl_DIR/re2_DIR pinning to Config.cmake.in so that
downstream consumers (e.g. api_client_example) using vcpkg also get the
correct OR-Tools co-installed absl/re2 instead of vcpkg's version.
On Windows, vcpkg builds Arrow as a shared library (DLLs), so only
Arrow::arrow_shared and Parquet::parquet_shared targets exist.
Use the ARROW_BUILD_STATIC generator expression (as recommended by
Arrow's own CMake usage docs) to select the correct target based on
how Arrow was built.
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
52.6% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

@guilpier-code

guilpier-code commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

Answers to the big comment above :

  1. Was addressed in merged PR 3654
  2. dynamic_cast chain with no registration mechanism : addressed in separate PR 3657
  3. OptimisationsSimulationTable no longer accumulates data across write() calls : it's a warning (we change of behavior), not a fix requirement.
  4. OptimisationsSimulationTable header has no namespace and a TODO comment : addressed in separate PR 3657
  5. ITableWriter::output_file_ is a non-const reference : Addressed in separate PR 3657
  6. makeTableWriter takes std::filesystem::path& (non-const reference) : addressed in separate PR 3657
  7. LegacySimulationTablesWriter uses std::invalid_argument for filesystem error : it's done in this PR
  8. LegacySimulationTablesWriter constructor takes const fs::path& but stores by copy : addressed in separate PR 3657

@guilpier-code guilpier-code requested a review from flomnes May 26, 2026 09:57
JasonMarechal25 and others added 8 commits May 26, 2026 13:10
Signed-off-by: GitHub Copilot <copilot@example.com>
Signed-off-by: GitHub Copilot <copilot@example.com>
Signed-off-by: GitHub Copilot <copilot@example.com>
Signed-off-by: GitHub Copilot <copilot@example.com>
Comment thread src/modeler/main.cpp Outdated
// Options parsing
std::filesystem::path studyPath(argv[1]);
Writer::TableFormat tableFormat = getTableFormat(argc, argv);
bool parquetFormat = parquetFormatRequired(argc, argv);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please keep format enum and not a boolean.

@guilpier-code guilpier-code May 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I knew you would say that, and I think you're right, I should have left it.
I wanted to make it as simple as possible by introducing a boolean, but it's less clear.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

guilpierre and others added 8 commits May 27, 2026 10:35
This reverts commit f0825a6.
Co-authored-by: guilpierre <guillaume.pierre_externe@rte-france.com>
…ests NR and make it point to the right commit
…for-legacy-solver

# Conflicts:
#	src/packaging/CMakeLists.txt
#	src/solver/simulation/economy.cpp
#	src/tests/resources/Antares_Simulator_Tests_NR
Signed-off-by: GitHub Copilot <copilot@example.com>
@sonarqubecloud

sonarqubecloud Bot commented Jun 1, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
48.7% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

@flomnes flomnes merged commit e51c283 into develop Jun 1, 2026
10 of 11 checks passed
@flomnes flomnes deleted the feature/parquet-for-legacy-solver branch June 1, 2026 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants