Harmonize, PR review

src/solver/optim-model-filler/ComponentFiller.cpp

@@ -148,8 +148,7 @@ void ComponentFiller::addVariables(Optimisation::LinearProblemApi::ILinearProble
         if (variable.isTimeDependent())
         {
             const Dimensions dim({},
-                                 Dimensions::TimeInterval(ctx.getFirstTimeStep(),
-                                                          ctx.getLastTimeStep()));
+                                 IntegerInterval(ctx.getFirstTimeStep(), ctx.getLastTimeStep()));
             // std::visit to handle the 4 cases: double/double, vector/double,
             // double/vector and vector/vector.
             std::visit(

src/solver/optim-model-filler/VariableDictionary.cpp

@@ -89,8 +89,30 @@ std::string buildVariableName(const PartialKey& key,
     return ret;
 }
 
-Dimensions::Dimensions(std::optional<int> nbScenarios, std::optional<TimeInterval> timeInterval):
-    nbScenarios(nbScenarios),
+IntegerInterval::Iterator::Iterator(int current):
+    current_(current)
+{
+}
+
+int IntegerInterval::Iterator::operator*() const
+{
+    return current_;
+}
+
+IntegerInterval::Iterator& IntegerInterval::Iterator::operator++()
+{ // Prefix increment
+    ++current_;
+    return *this;
+}
+
+bool IntegerInterval::Iterator::operator!=(const Iterator& other) const
+{
+    return current_ != other.current_;
+}
+
+Dimensions::Dimensions(std::optional<IntegerInterval> scenarioInterval,
+                       std::optional<IntegerInterval> timeInterval):
+    scenarioInterval(scenarioInterval),
     timeInterval(timeInterval)
 {
 }
@@ -102,43 +124,17 @@ bool Dimensions::isTimeDependent() const
 
 bool Dimensions::isScenarioDependent() const
 {
-    return nbScenarios.has_value();
+    return scenarioInterval.has_value();
 }
 
-std::vector<int> Dimensions::getTimesteps() const
+IntegerInterval Dimensions::getTimesteps() const
 {
-    if (timeInterval)
-    {
-        std::vector<int> ret(timeInterval->finalTime - timeInterval->initialTime + 1);
-        for (int t = timeInterval->initialTime; t <= timeInterval->finalTime; ++t)
-        {
-            ret[t - timeInterval->initialTime] = t;
-        }
-        return ret;
-    }
-    else
-    {
-        // Arbitrary
-        return {0};
-    }
+    return timeInterval.value_or(IntegerInterval{});
 }
 
-std::vector<int> Dimensions::getScenarioIndices() const
+IntegerInterval Dimensions::getScenarioIndices() const
 {
-    if (nbScenarios)
-    {
-        std::vector<int> ret(*nbScenarios);
-        for (int s = 0; s < *nbScenarios; ++s)
-        {
-            ret[s] = s;
-        }
-        return ret;
-    }
-    else
-    {
-        // Arbitrary
-        return {0};
-    }
+    return scenarioInterval.value_or(IntegerInterval{});
 }
 
 int Dimensions::getNumberOfTimesteps() const
@@ -168,30 +164,32 @@ void VariableDictionary::addVariable(const Dimensions& dimensions,
     auto& m = hmv[key];
     const auto scenarios = dimensions.getScenarioIndices();
     const auto timesteps = dimensions.getTimesteps();
-    const int offset = timesteps.front();
+    const int offset = *timesteps.begin();
     m.resize(scenarios.size());
     for (int scenario: scenarios)
     {
         m[scenario].resize(timesteps.size());
 
         for (int timestep: timesteps)
         {
-            const auto ts = buildOptional(dimensions.isTimeDependent(), timestep);
             const auto sc = buildOptional(dimensions.isScenarioDependent(), scenario);
+            const auto ts = buildOptional(dimensions.isTimeDependent(), timestep);
             const std::string name = buildVariableName(key, sc, ts);
-            m[scenario][timestep - offset] = func(timestep, scenario, name);
+            m[scenario][timestep - offset] = func(scenario, timestep, name);
         }
     }
 }
 
 VariableDictionary::Value VariableDictionary::operator[](const FullKey& k) const
 {
-    return hmv.at(k.getPartialKey())[k.getScenario().value_or(0)][k.getTimestep().value_or(0)];
+    return hmv.at(k.getPartialKey())
+      .at(k.getScenario().value_or(0))
+      .at(k.getTimestep().value_or(0));
 }
 
 VariableDictionary::Value& VariableDictionary::operator[](const FullKey& k)
 {
-    return hmv[k.getPartialKey()][k.getScenario().value_or(0)][k.getTimestep().value_or(0)];
+    return hmv[k.getPartialKey()].at(k.getScenario().value_or(0)).at(k.getTimestep().value_or(0));
 }
 
 const VariableDictionary::TwoIndexVector& VariableDictionary::operator[](const PartialKey& k) const
@@ -202,29 +200,29 @@ const VariableDictionary::TwoIndexVector& VariableDictionary::operator[](const P
 VariableDictionary::Value VariableDictionary::operator()(const std::string& component,
                                                          const std::string& variable) const
 {
-    return hmv.at(PartialKey(component, variable))[0][0];
+    return hmv.at(PartialKey(component, variable)).at(0).at(0);
 }
 
 VariableDictionary::Value& VariableDictionary::operator()(const std::string& component,
                                                           const std::string& variable)
 {
-    return hmv.at(PartialKey(component, variable))[0][0];
+    return hmv.at(PartialKey(component, variable)).at(0).at(0);
 }
 
 VariableDictionary::Value VariableDictionary::operator()(const std::string& component,
                                                          const std::string& variable,
                                                          int scenario,
                                                          int timestep) const
 {
-    return hmv.at(PartialKey(component, variable))[scenario][timestep];
+    return hmv.at(PartialKey(component, variable)).at(scenario).at(timestep);
 }
 
 VariableDictionary::Value& VariableDictionary::operator()(const std::string& component,
                                                           const std::string& variable,
                                                           int scenario,
                                                           int timestep)
 {
-    return hmv[PartialKey(component, variable)][scenario][timestep];
+    return hmv[PartialKey(component, variable)].at(scenario).at(timestep);
 }
 
 } // namespace Antares::Optimization

src/solver/optim-model-filler/include/antares/solver/optim-model-filler/VariableDictionary.h

@@ -53,25 +53,54 @@ class hash
     std::size_t operator()(const PartialKey& p) const;
 };
 
-class Dimensions
+struct IntegerInterval
 {
-public:
-    struct TimeInterval
+    int initialTime = 0;
+    int finalTime = 0;
+
+    class Iterator
     {
-        int initialTime;
-        int finalTime;
+    public:
+        Iterator(int current);
+        int operator*() const;
+        Iterator& operator++();
+        bool operator!=(const Iterator& other) const;
+
+    private:
+        int current_;
     };
 
-    Dimensions(std::optional<int> nbScenarios, std::optional<TimeInterval> timeInterval);
+    Iterator begin() const
+    {
+        return Iterator(initialTime);
+    }
+
+    Iterator end() const
+    {
+        return Iterator(finalTime + 1);
+    } // Make it inclusive
+
+    std::size_t size() const
+    {
+        return finalTime - initialTime + 1;
+    }
+};
+
+class Dimensions
+{
+public:
+    Dimensions() = default;
+    Dimensions(std::optional<IntegerInterval> scenarioInterval,
+               std::optional<IntegerInterval> timeInterval);
     bool isTimeDependent() const;
     bool isScenarioDependent() const;
-    std::vector<int> getTimesteps() const;
-    std::vector<int> getScenarioIndices() const;
+    IntegerInterval getTimesteps() const;
+    IntegerInterval getScenarioIndices() const;
     int getNumberOfTimesteps() const;
 
 private:
-    std::optional<int> nbScenarios;
-    std::optional<TimeInterval> timeInterval;
+    std::optional<IntegerInterval> scenarioInterval;
+    std::optional<IntegerInterval> timeInterval;
 };
 
 class VariableDictionary

src/tests/src/solver/optim-model-filler/test_VariableDictionary.cpp

@@ -21,23 +21,127 @@
 
 #define WIN32_LEAN_AND_MEAN
 
+#include <map>
+
 #include <boost/test/test_tools.hpp>
 #include <boost/test/unit_test.hpp>
 
 #include "antares/solver/optim-model-filler/ComponentFiller.h"
 
-using Dimensions = Antares::Optimization::Dimensions;
+using namespace Antares::Optimization;
 
 BOOST_AUTO_TEST_SUITE(DimensionsSuite)
 
+BOOST_AUTO_TEST_CASE(PartialKeyGetters)
+{
+    PartialKey pk("component", "variable");
+    BOOST_CHECK_EQUAL(pk.getComponent(), "component");
+    BOOST_CHECK_EQUAL(pk.getVariable(), "variable");
+}
+
+BOOST_AUTO_TEST_CASE(FullKeyGetters_2ArgsConstructor)
+{
+    FullKey k("component", "variable");
+    BOOST_CHECK_EQUAL(k.getComponent(), "component");
+    BOOST_CHECK_EQUAL(k.getVariable(), "variable");
+
+    BOOST_CHECK(!k.getScenario());
+    BOOST_CHECK(!k.getTimestep());
+}
+
+BOOST_AUTO_TEST_CASE(FullKeyGetters_4ArgsConstructor)
+{
+    FullKey k("component", "variable", 3, 4);
+    BOOST_CHECK_EQUAL(k.getComponent(), "component");
+    BOOST_CHECK_EQUAL(k.getVariable(), "variable");
+
+    BOOST_CHECK_EQUAL(*k.getScenario(), 3);
+    BOOST_CHECK_EQUAL(*k.getTimestep(), 4);
+}
+
+BOOST_AUTO_TEST_CASE(FullKeyCompare)
+{
+    FullKey k1("component", "a");
+    FullKey k2("component", "a");
+    BOOST_CHECK(k1 == k2);
+    BOOST_CHECK(k1 <= k2);
+
+    FullKey k3("component", "b");
+    BOOST_CHECK(k3 != k1);
+    BOOST_CHECK(k1 < k3);
+
+    FullKey k4("komponent", "a");
+    BOOST_CHECK(k4 != k1);
+}
+
+BOOST_AUTO_TEST_CASE(IntegerInterval_count)
+{
+    int count = 0;
+    Antares::Optimization::IntegerInterval interval(0, 2);
+    BOOST_CHECK_EQUAL(interval.size(), 3);
+    for (int x: interval)
+    {
+        count++;
+    }
+    BOOST_CHECK_EQUAL(count, 3);
+}
+
 BOOST_AUTO_TEST_CASE(no_scenarios)
 {
-    Dimensions dim({}, Dimensions::TimeInterval(0, 2));
+    Dimensions dim({}, Antares::Optimization::IntegerInterval(0, 2));
     BOOST_CHECK_EQUAL(dim.getNumberOfTimesteps(), 3);
-    const std::vector<int> expected_ts{0, 1, 2};
-    const std::vector<int> ts = dim.getTimesteps();
-    BOOST_CHECK_EQUAL_COLLECTIONS(expected_ts.begin(), expected_ts.end(), ts.begin(), ts.end());
+}
+
+std::pair<std::map<std::pair<int, int>, std::string>, VariableDictionary> namesFromDimensions(
+  const Dimensions& dim)
+{
+    VariableDictionary vdict;
+    std::map<std::pair<int, int>, std::string> names;
+    vdict.addVariable(dim,
+                      PartialKey("component", "variable"),
+                      [&names](int sc, int ts, const std::string& name)
+                      {
+                          names[std::pair(sc, ts)] = name;
+                          return nullptr;
+                      });
+    return {names, vdict};
+}
+
+BOOST_AUTO_TEST_CASE(addVariable_no_ts_no_sc)
+{
+    const auto [names, dict] = namesFromDimensions({});
+    BOOST_CHECK_EQUAL(names.size(), 1);
+    BOOST_CHECK_EQUAL(names.at(std::pair(0, 0)), "component.variable");
+
+    BOOST_CHECK_NO_THROW(dict("component", "variable"));
+}
+
+BOOST_AUTO_TEST_CASE(addVariable_no_ts_multiple_sc)
+{
+    const auto [names, dict] = namesFromDimensions({IntegerInterval(0, 2), {}});
+    BOOST_CHECK_EQUAL(names.size(), 3);
+    BOOST_CHECK_EQUAL(names.at(std::pair(0, 0)), "component.variable_s0");
+
+    BOOST_CHECK_NO_THROW(dict("component", "variable", 1, 0));
+}
+
+BOOST_AUTO_TEST_CASE(addVariable_multiple_ts_no_sc)
+{
+    const auto [names, dict] = namesFromDimensions({{}, IntegerInterval(0, 2)});
+    BOOST_CHECK_EQUAL(names.size(), 3);
+    BOOST_CHECK_EQUAL(names.at(std::pair(0, 0)), "component.variable_t0");
+
+    BOOST_CHECK_NO_THROW(dict("component", "variable", 0, 2));
+}
+
+BOOST_AUTO_TEST_CASE(addVariable_multiple_ts_sc)
+{
+    const auto [names, dict] = namesFromDimensions({IntegerInterval(0, 2), IntegerInterval(0, 4)});
+    BOOST_CHECK_EQUAL(names.at(std::pair(0, 0)), "component.variable_s0_t0");
+    BOOST_CHECK_EQUAL(names.at(std::pair(2, 3)), "component.variable_s2_t3");
+    BOOST_CHECK(!names.contains(std::pair(3, 3)));
 
-    BOOST_CHECK(dim.getScenarioIndices() == std::vector<int>{0});
+    BOOST_CHECK_THROW(dict("component", "variable", 3, 2), std::out_of_range);
+    BOOST_CHECK_THROW(dict("component", "variable", 2, 5), std::out_of_range);
 }
 BOOST_AUTO_TEST_SUITE_END()