From 302c7ac2086c26b1ac4f2976c400e3e5a176e0de Mon Sep 17 00:00:00 2001 From: Florian OMNES Date: Fri, 20 Dec 2024 16:10:46 +0100 Subject: [PATCH 1/7] High-level API : provide simulation, don't return it --- src/api/API.cpp | 14 +++++++----- .../include/antares/api/SimulationResults.h | 10 ++++----- src/api/include/antares/api/solver.h | 1 + src/api/private/API.h | 2 ++ src/api/solver.cpp | 5 +++-- src/api_client_example/src/API_client.cpp | 5 +++-- src/tests/src/api_internal/test_api.cpp | 22 +++++-------------- src/tests/src/api_lib/test_api.cpp | 2 +- 8 files changed, 28 insertions(+), 33 deletions(-) diff --git a/src/api/API.cpp b/src/api/API.cpp index 7bcda34502..62e599c166 100644 --- a/src/api/API.cpp +++ b/src/api/API.cpp @@ -34,6 +34,7 @@ namespace Antares::API { SimulationResults APIInternal::run( const IStudyLoader& study_loader, + const std::filesystem::path& output, const Antares::Solver::Optimization::OptimizationOptions& optOptions) { try @@ -43,9 +44,9 @@ SimulationResults APIInternal::run( catch (const ::Antares::Error::StudyFolderDoesNotExist& e) { Antares::API::Error err{.reason = e.what()}; - return {.simulationPath = "", .antares_problems = {}, .error = err}; + return {.antares_problems = {}, .error = err}; } - return execute(optOptions); + return execute(output, optOptions); } /** @@ -56,6 +57,7 @@ SimulationResults APIInternal::run( * dupllication */ SimulationResults APIInternal::execute( + const std::filesystem::path& output, const Antares::Solver::Optimization::OptimizationOptions& optOptions) const { // study_ == nullptr e.g when the -h flag is given @@ -63,13 +65,15 @@ SimulationResults APIInternal::execute( { using namespace std::string_literals; Antares::API::Error err{.reason = "Couldn't create study"s}; - return {.simulationPath{}, .antares_problems{}, .error = err}; + return {.antares_problems{}, .error = err}; } Settings settings; auto& parameters = study_->parameters; parameters.optOptions = optOptions; + study_->folderOutput = output; + Benchmarking::DurationCollector durationCollector; Benchmarking::OptimizationInfo optimizationInfo; auto ioQueueService = std::make_shared(); @@ -90,8 +94,6 @@ SimulationResults APIInternal::execute( // Importing Time-Series if asked study_->importTimeseriesIntoInput(); - return {.simulationPath = study_->folderOutput, - .antares_problems = simulationObserver.acquireLps(), - .error{}}; + return {.antares_problems = simulationObserver.acquireLps(), .error{}}; } } // namespace Antares::API diff --git a/src/api/include/antares/api/SimulationResults.h b/src/api/include/antares/api/SimulationResults.h index 5a5e93982a..31f0e9aec0 100644 --- a/src/api/include/antares/api/SimulationResults.h +++ b/src/api/include/antares/api/SimulationResults.h @@ -23,6 +23,7 @@ #include #include #include + #include "antares/solver/lps/LpsFromAntares.h" namespace Antares::API @@ -31,7 +32,8 @@ namespace Antares::API * @struct Error * @brief The Error structure is used to represent an error that occurred during the simulation. */ -struct Error { +struct Error +{ /** * @brief The reason for the error. */ @@ -45,10 +47,6 @@ struct Error { */ struct [[nodiscard("Contains results and potential error")]] SimulationResults { - /** - * @brief The path to the simulation (output). - */ - std::filesystem::path simulationPath; /** * @brief weekly problems */ @@ -59,4 +57,4 @@ struct [[nodiscard("Contains results and potential error")]] SimulationResults std::optional error; }; -} \ No newline at end of file +} // namespace Antares::API diff --git a/src/api/include/antares/api/solver.h b/src/api/include/antares/api/solver.h index a8279c00c8..64fa13ac55 100644 --- a/src/api/include/antares/api/solver.h +++ b/src/api/include/antares/api/solver.h @@ -36,5 +36,6 @@ namespace Antares::API */ SimulationResults PerformSimulation( const std::filesystem::path& study_path, + const std::filesystem::path& output, const Antares::Solver::Optimization::OptimizationOptions& optOptions) noexcept; } // namespace Antares::API diff --git a/src/api/private/API.h b/src/api/private/API.h index 3561f6d21b..c52c3304bf 100644 --- a/src/api/private/API.h +++ b/src/api/private/API.h @@ -48,11 +48,13 @@ class APIInternal * @return SimulationResults object which contains the results of the simulation. */ SimulationResults run(const IStudyLoader& study_loader, + const std::filesystem::path& output, const Antares::Solver::Optimization::OptimizationOptions& optOptions); private: std::shared_ptr study_; SimulationResults execute( + const std::filesystem::path& output, const Antares::Solver::Optimization::OptimizationOptions& optOptions) const; }; diff --git a/src/api/solver.cpp b/src/api/solver.cpp index 4e65696f8a..ac6810e7f1 100644 --- a/src/api/solver.cpp +++ b/src/api/solver.cpp @@ -30,18 +30,19 @@ namespace Antares::API SimulationResults PerformSimulation( const std::filesystem::path& study_path, + const std::filesystem::path& output, const Antares::Solver::Optimization::OptimizationOptions& optOptions) noexcept { try { APIInternal api; FileTreeStudyLoader study_loader(study_path); - return api.run(study_loader, optOptions); + return api.run(study_loader, output, optOptions); } catch (const std::exception& e) { Antares::API::Error err{.reason = e.what()}; - return SimulationResults{.simulationPath = study_path, .antares_problems{}, .error = err}; + return SimulationResults{.antares_problems{}, .error = err}; } } diff --git a/src/api_client_example/src/API_client.cpp b/src/api_client_example/src/API_client.cpp index 9cb19d8e59..8ebba07042 100644 --- a/src/api_client_example/src/API_client.cpp +++ b/src/api_client_example/src/API_client.cpp @@ -23,7 +23,8 @@ #include -Antares::API::SimulationResults solve(std::filesystem::path study_path) +Antares::API::SimulationResults solve(std::filesystem::path study_path, + std::filesystem::path output) { - return Antares::API::PerformSimulation(std::move(study_path), {}); + return Antares::API::PerformSimulation(std::move(study_path), std::move(output), {}); } diff --git a/src/tests/src/api_internal/test_api.cpp b/src/tests/src/api_internal/test_api.cpp index 5214aa8c10..01a191fd26 100644 --- a/src/tests/src/api_internal/test_api.cpp +++ b/src/tests/src/api_internal/test_api.cpp @@ -52,28 +52,17 @@ class InMemoryStudyLoader: public Antares::IStudyLoader builder.study->initializeRuntimeInfos(); builder.setNumberMCyears(1); builder.study->parameters.resultFormat = ResultFormat::inMemory; - builder.study->prepareOutput(); return std::move(builder.study); } bool success_ = true; }; -BOOST_AUTO_TEST_CASE(simulation_path_points_to_results) -{ - Antares::API::APIInternal api; - auto study_loader = std::make_unique(); - auto results = api.run(*study_loader.get(), {}); - BOOST_CHECK_EQUAL(results.simulationPath, std::filesystem::path{"no_output"}); - // Testing for "no_output" is a bit weird, but it's the only way to test this without actually - // running the simulation -} - BOOST_AUTO_TEST_CASE(api_run_contains_antares_problem) { Antares::API::APIInternal api; auto study_loader = std::make_unique(); - auto results = api.run(*study_loader.get(), {}); + auto results = api.run(*study_loader.get(), {}, {}); BOOST_CHECK(!results.antares_problems.empty()); BOOST_CHECK(!results.error); @@ -83,7 +72,7 @@ BOOST_AUTO_TEST_CASE(result_failure_when_study_is_null) { Antares::API::APIInternal api; auto study_loader = std::make_unique(false); - auto results = api.run(*study_loader.get(), {}); + auto results = api.run(*study_loader.get(), {}, {}); BOOST_CHECK(results.error); } @@ -93,7 +82,7 @@ BOOST_AUTO_TEST_CASE(result_contains_problems) { Antares::API::APIInternal api; auto study_loader = std::make_unique(); - auto results = api.run(*study_loader.get(), {}); + auto results = api.run(*study_loader.get(), {}, {}); BOOST_CHECK(!results.antares_problems.empty()); BOOST_CHECK(!results.error); @@ -109,7 +98,7 @@ BOOST_AUTO_TEST_CASE(result_with_ortools_coin) .solverLogs = false, .solverParameters = ""}; - auto results = api.run(*study_loader.get(), opt); + auto results = api.run(*study_loader.get(), {}, opt); BOOST_CHECK(!results.antares_problems.empty()); BOOST_CHECK(!results.error); @@ -126,7 +115,8 @@ BOOST_AUTO_TEST_CASE(invalid_ortools_solver) .solverLogs = true, .solverParameters = ""}; - auto shouldThrow = [&api, &study_loader, &opt] { return api.run(*study_loader.get(), opt); }; + auto shouldThrow = [&api, &study_loader, &opt] + { return api.run(*study_loader.get(), {}, opt); }; BOOST_CHECK_EXCEPTION(shouldThrow(), std::invalid_argument, checkMessage("Solver this-solver-does-not-exist not found")); diff --git a/src/tests/src/api_lib/test_api.cpp b/src/tests/src/api_lib/test_api.cpp index c1111b5c6a..8460c4533a 100644 --- a/src/tests/src/api_lib/test_api.cpp +++ b/src/tests/src/api_lib/test_api.cpp @@ -29,7 +29,7 @@ BOOST_AUTO_TEST_CASE(result_failure_when_study_path_invalid) { using namespace std::string_literals; - auto results = Antares::API::PerformSimulation("dummy"s, {}); + auto results = Antares::API::PerformSimulation("dummy"s, {}, {}); BOOST_CHECK(results.error); BOOST_CHECK(!results.error->reason.empty()); } From 3e89d09526bab5690694d968a44ff68aca7df91f Mon Sep 17 00:00:00 2001 From: Florian OMNES Date: Fri, 20 Dec 2024 16:12:11 +0100 Subject: [PATCH 2/7] Add api to formatting script --- src/format-code.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/format-code.sh b/src/format-code.sh index abac85a423..752080ca58 100755 --- a/src/format-code.sh +++ b/src/format-code.sh @@ -3,7 +3,7 @@ if [ $# -eq 0 ] then # No arguments: format all - SOURCE_DIRS="analyzer/ libs/ solver/ tools/ config/ tests/ packaging/" + SOURCE_DIRS="analyzer/ libs/ solver/ tools/ config/ tests/ packaging/ api/" SOURCE_FILES=$(find $SOURCE_DIRS -regextype egrep -regex ".*/*\.(c|cxx|cpp|cc|h|hxx|hpp)$" ! -path '*/antlr-interface/*') else # Format files provided as arguments From 28a2ad594363749fbe3abf760fe40d40f9144859 Mon Sep 17 00:00:00 2001 From: Florian OMNES Date: Fri, 20 Dec 2024 16:56:32 +0100 Subject: [PATCH 3/7] Fix --- src/api_client_example/tests/test.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/api_client_example/tests/test.cpp b/src/api_client_example/tests/test.cpp index 4adee7bfd6..bd5b3d86a8 100644 --- a/src/api_client_example/tests/test.cpp +++ b/src/api_client_example/tests/test.cpp @@ -22,10 +22,12 @@ #define BOOST_TEST_MODULE test_client_api #include + #include "API_client.h" -BOOST_AUTO_TEST_CASE(test_run) { - auto results = solve("dummy_study_test_client_api"); +BOOST_AUTO_TEST_CASE(test_run) +{ + auto results = solve("dummy_study_test_client_api", {}); BOOST_CHECK(results.error); BOOST_CHECK(!results.error->reason.empty()); auto c = results.error->reason; @@ -34,4 +36,4 @@ BOOST_AUTO_TEST_CASE(test_run) { BOOST_CHECK(results.error->reason.find("folder") != std::string::npos); BOOST_CHECK(results.error->reason.find("not") != std::string::npos); BOOST_CHECK(results.error->reason.find("exist") != std::string::npos); -} \ No newline at end of file +} From 3787ea6df94899a6a6c03e5ff4897cc4ffccae8a Mon Sep 17 00:00:00 2001 From: Florian OMNES Date: Tue, 31 Dec 2024 16:06:28 +0100 Subject: [PATCH 4/7] Move call to saveAboutTheStudy to avoid output dir duplication --- src/api/API.cpp | 6 ++++-- src/solver/application/application.cpp | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/api/API.cpp b/src/api/API.cpp index 62e599c166..60fc9d8471 100644 --- a/src/api/API.cpp +++ b/src/api/API.cpp @@ -72,17 +72,19 @@ SimulationResults APIInternal::execute( auto& parameters = study_->parameters; parameters.optOptions = optOptions; - study_->folderOutput = output; - Benchmarking::DurationCollector durationCollector; Benchmarking::OptimizationInfo optimizationInfo; auto ioQueueService = std::make_shared(); ioQueueService->maximumThreadCount(1); ioQueueService->start(); + + study_->folderOutput = output; auto resultWriter = Solver::resultWriterFactory(parameters.resultFormat, study_->folderOutput, ioQueueService, durationCollector); + study_->saveAboutTheStudy(*resultWriter); + SimulationObserver simulationObserver; optimizationInfo = simulationRun(*study_, diff --git a/src/solver/application/application.cpp b/src/solver/application/application.cpp index 2d1695bcf0..43dfa3368d 100644 --- a/src/solver/application/application.cpp +++ b/src/solver/application/application.cpp @@ -161,9 +161,6 @@ void Application::readDataForTheStudy(Data::StudyLoadOptions& options) Antares::Solver::initializeSignalHandlers(resultWriter); - // Save about-the-study files (comments, notes, etc.) - study.saveAboutTheStudy(*resultWriter); - // Name of the simulation (again, if the value has been overwritten) if (!pSettings.simulationName.empty()) { @@ -375,6 +372,9 @@ void Application::execute() return; } + // Save about-the-study files (comments, notes, etc.) + pStudy->saveAboutTheStudy(*resultWriter); + SystemMemoryLogger memoryReport; memoryReport.interval(1000 * 60 * 5); // 5 minutes memoryReport.start(); From 8d1973657416aeaae0795572671fe314e25a23f3 Mon Sep 17 00:00:00 2001 From: Florian OMNES Date: Tue, 31 Dec 2024 16:51:59 +0100 Subject: [PATCH 5/7] Fix API_client.h --- src/api_client_example/src/API_client.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/api_client_example/src/API_client.h b/src/api_client_example/src/API_client.h index 5d8500649e..7f77a26e7f 100644 --- a/src/api_client_example/src/API_client.h +++ b/src/api_client_example/src/API_client.h @@ -22,7 +22,8 @@ #pragma once -#include #include +#include -Antares::API::SimulationResults solve(std::filesystem::path study_path); +Antares::API::SimulationResults solve(std::filesystem::path study_path, + std::filesystem::path output); From 04b5f05a0469c7a249b0ba1f336b1b7e3da858b0 Mon Sep 17 00:00:00 2001 From: Florian OMNES Date: Tue, 31 Dec 2024 17:37:21 +0100 Subject: [PATCH 6/7] Remove get() --- src/tests/src/api_internal/test_api.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/tests/src/api_internal/test_api.cpp b/src/tests/src/api_internal/test_api.cpp index 01a191fd26..47034cb19d 100644 --- a/src/tests/src/api_internal/test_api.cpp +++ b/src/tests/src/api_internal/test_api.cpp @@ -62,7 +62,7 @@ BOOST_AUTO_TEST_CASE(api_run_contains_antares_problem) { Antares::API::APIInternal api; auto study_loader = std::make_unique(); - auto results = api.run(*study_loader.get(), {}, {}); + auto results = api.run(*study_loader, {}, {}); BOOST_CHECK(!results.antares_problems.empty()); BOOST_CHECK(!results.error); @@ -72,7 +72,7 @@ BOOST_AUTO_TEST_CASE(result_failure_when_study_is_null) { Antares::API::APIInternal api; auto study_loader = std::make_unique(false); - auto results = api.run(*study_loader.get(), {}, {}); + auto results = api.run(*study_loader, {}, {}); BOOST_CHECK(results.error); } @@ -82,7 +82,7 @@ BOOST_AUTO_TEST_CASE(result_contains_problems) { Antares::API::APIInternal api; auto study_loader = std::make_unique(); - auto results = api.run(*study_loader.get(), {}, {}); + auto results = api.run(*study_loader, {}, {}); BOOST_CHECK(!results.antares_problems.empty()); BOOST_CHECK(!results.error); @@ -98,7 +98,7 @@ BOOST_AUTO_TEST_CASE(result_with_ortools_coin) .solverLogs = false, .solverParameters = ""}; - auto results = api.run(*study_loader.get(), {}, opt); + auto results = api.run(*study_loader, {}, opt); BOOST_CHECK(!results.antares_problems.empty()); BOOST_CHECK(!results.error); @@ -115,8 +115,7 @@ BOOST_AUTO_TEST_CASE(invalid_ortools_solver) .solverLogs = true, .solverParameters = ""}; - auto shouldThrow = [&api, &study_loader, &opt] - { return api.run(*study_loader.get(), {}, opt); }; + auto shouldThrow = [&api, &study_loader, &opt] { return api.run(*study_loader, {}, opt); }; BOOST_CHECK_EXCEPTION(shouldThrow(), std::invalid_argument, checkMessage("Solver this-solver-does-not-exist not found")); From e21ceeae25f7abf7664011a58e32da4ae76d8607 Mon Sep 17 00:00:00 2001 From: Florian OMNES Date: Tue, 31 Dec 2024 17:37:31 +0100 Subject: [PATCH 7/7] Only save results if output is not empty --- src/api/API.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/api/API.cpp b/src/api/API.cpp index 60fc9d8471..63b4a9fda8 100644 --- a/src/api/API.cpp +++ b/src/api/API.cpp @@ -83,7 +83,12 @@ SimulationResults APIInternal::execute( study_->folderOutput, ioQueueService, durationCollector); - study_->saveAboutTheStudy(*resultWriter); + + // In some cases (e.g tests) we don't want to write anything + if (!output.empty()) + { + study_->saveAboutTheStudy(*resultWriter); + } SimulationObserver simulationObserver;