From 9a9715ebdba370b59bf8aa637071b68abf2f2733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Omn=C3=A8s?= Date: Fri, 3 Jan 2025 09:43:05 +0100 Subject: [PATCH] High-level API: provide output path instead of returning it [ANT-2561] (#2548) TODO - [x] Fix build for api_client_test (missing argument) - [x] Fix extra output directory created by Simulator, for example 20241204-1554eco and 20241204-1554eco-2 --- src/api/API.cpp | 21 +++++++++++++------ .../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/api_client_example/src/API_client.h | 5 +++-- src/api_client_example/tests/test.cpp | 8 ++++--- src/format-code.sh | 2 +- src/solver/application/application.cpp | 6 +++--- src/tests/src/api_internal/test_api.cpp | 21 +++++-------------- src/tests/src/api_lib/test_api.cpp | 2 +- 12 files changed, 46 insertions(+), 42 deletions(-) diff --git a/src/api/API.cpp b/src/api/API.cpp index 7bcda34502..63b4a9fda8 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,7 +65,7 @@ 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; @@ -75,10 +77,19 @@ SimulationResults APIInternal::execute( auto ioQueueService = std::make_shared(); ioQueueService->maximumThreadCount(1); ioQueueService->start(); + + study_->folderOutput = output; auto resultWriter = Solver::resultWriterFactory(parameters.resultFormat, study_->folderOutput, ioQueueService, durationCollector); + + // In some cases (e.g tests) we don't want to write anything + if (!output.empty()) + { + study_->saveAboutTheStudy(*resultWriter); + } + SimulationObserver simulationObserver; optimizationInfo = simulationRun(*study_, @@ -90,8 +101,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/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); 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 +} 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 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(); diff --git a/src/tests/src/api_internal/test_api.cpp b/src/tests/src/api_internal/test_api.cpp index 5214aa8c10..47034cb19d 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, {}, {}); 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, {}, {}); 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, {}, {}); 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, {}, opt); BOOST_CHECK(!results.antares_problems.empty()); BOOST_CHECK(!results.error); @@ -126,7 +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")); 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()); }