Scenario builder 1 : interface is not handy

[guilpier-code]

This page discusses a particular aspect of this more general discussion

Please refer to this general discussion for definitions or elements of context.

How scenario builder is currently created and used in tests

Currently, here is how we create and use the scenario builder from end-to-end tests or from Antares itself (see comments on code excerpts right after) :

// Utils.h
...
class ScenarioBuilderRule
{
public:
    ScenarioBuilderRule(Study& study);
    loadTSNumberData& load() { return rules_->load; }
    BindingConstraintsTSNumberData& bcGroup() { return rules_->binding_constraints; }

private:
    Rules::Ptr rules_;
};

// Utils.cpp
...
ScenarioBuilderRule::ScenarioBuilderRule(Study& study)
{
    study.scenarioRulesCreate();
    ScenarioBuilder::Sets* sets = study.scenarioRules;
    if (sets && !sets->empty())
    {
        rules_ = sets->createNew("Custom");

        study.parameters.useCustomScenario = true;
        study.parameters.activeRulesScenario = "Custom";
    }
 }

// From tests
    ...
    ScenarioBuilderRule scenarioBuilderRule(*study);
    scenarioBuilderRule.bcGroup().setTSnumber(BC->group(), 0, 1);

Comments on previous code

• Note that class ScenarioBuilderRule was defined only in order to ease the use of scenario builder in tests. The production code available to create and configure a scenario builder can be found in the ScenarioBuilderRule's constructor.
• In the above code, class ScenarioBuilder::Sets contains a list of all the scenario builder rule sets : here the names are very confusing. Actually, class Sets is a set of rule sets (we point this in next section)

Possible flaws in this code

• Class ScenarioBuilderRule was created because the production code related to scenario builder is not well designed enough for an easy use. We should try to change the production code in such a way we don't need this class.
• Class Sets (see sets.h/.hxx/.cpp) is a collection of rule sets, and the class for a rule set is named Rules (see rules.h/.hxx/.cpp). This is confusing. If we can, we should clarify this.
• Utils.cpp : study.scenarioRulesCreate(); :
  • Study (whose role, at most, is to contain input data, but not to contain objects built from input data [like the scenario builder]) should have nothing to do with the creation of a list of scenario builder rules.
  • Furthermore, creation of an instance should be created with a constructor whenever possible, otherwise what are the constructors for ?
• Utils.cpp : ScenarioBuilder::Sets* sets = study.scenarioRules; : the consequence of the study containing the list of rules is this line, which could be avoided.
• Utils.cpp : rules_ = sets->createNew("Custom"); : here we call a create method on class Sets to create a rule. We should first create a rule with a class Rules constructor and add it to an instance of Sets (as recommended in this discussion on new API
• Active rule : the active scenario builder rule is stored in the yuni string study.parameters.activeRulesScenario. It seems to me that the piece of information about which rule is active should be contained inside the list of rules, rather than in the study.
• Same as the previous remark, but about boolean study.parameters.useCustomScenario

[guilpier-code]

How scenario builder could be created and used

A new design of scenario builder will have consequences on end-to-end tests and on the production code. So we suggest changes in the scenario builder use for both.
What we're trying to do here :

• Isolate the scenario builder from the rest of the world (so that it can be possibly tested in isolation)
• Design a simple and easy to use interface for the scenario builder
• Internally designed in such a way we can easily add a category (example : add "final hydro level" as a category)
• Reduce the scenario builder code flaws listed above

End-to-end tests

// From tests
   ...
   ScenarioBuilder::Rules rules("Custom rules", study);
   rules.category("BCgroups")
        .location(BC->group())  // Group name
        .year(0)
        .setValue(1);
   // In case we want choose a TS number for a cluster
   rules.category("Thermal")
        .location("Area 1")
        .location("my cluster")  // Location has 2 coordinates in such a case
        .year(2)
        .setValue(5);
  // NTC
   rules.category("NTC")
        .location("Area 1")
        .location("Area 2")  // For a link, location has 2 coordinates in such a case
        .year(2)
        .setValue(5);
   
   rules.apply();

Remarks :

• the study as argument of the constructor is used to store internally a reference to each simulation 2d-matrix, so that we can update them when calling the apply() method. Note that this constructor argument prevents the object rules to be isolated from the rest of the world and from being tested in isolation. This flaw has already been raised in the main discussion.
• The internal design of class ScenarioBuilder::Rules is more the matter of discussion 1574
• Note that we don't need the set of rules anymore in a test (ScenarioBuilder::Sets), because calling the apply() method on a specific rule is enough to take it into account.
• Note that, using this interface, we do not use a location number anymore (area number, link number, cluster number, ...), but a location name instead. In the end-to-end tests, it's makes code more clear. We bet it will be the same (or at least will do no harm) in the production code.
  That said, doing so could turn out to be more difficult to implement, so we may pass a location number eventually.
• Moreover, what happens if client code makes a mistake in the location name, or category name ? Exceptions ? Boolean return code ?

In production code
If we change the class Rules interface as in previous excerpts, there are few places in the production code that will change.

• Reading the scenario builder data :

// rules.cpp
bool Rules::readSolar(const AreaName::Vector& splitKey, String value, bool updaterMode)
{
    const uint year = splitKey[2].to<uint>();
    const AreaName& areaname = splitKey[1];
    const Data::Area* area = getArea(areaname, updaterMode);
    ...
    uint val = fromStringToTSnumber(value);
    category("Solar").location(area->name).year(2).setValue(val);  // <-- change here 
    return true;
}

• Updating the simulation 2d-matrices with scenario builder (the apply step)
  Currently, 3 things are done in the ISimulation<Impl>::run() method :
  • Resizing the simulation 2d-matrices
  • Fill these 2d-matrices with random numbers
  • Possibly copy the values held by the scenario builder into the simulation 2d-matrices

 study.resizeAllTimeseriesNumbers(1 + study.runtime->rangeLimits.year[Data::rangeEnd]);
 if (not TimeSeriesNumbers::Generate(study))
 {
     throw FatalError("An unrecoverable error has occured. Can not continue.");
 }
 if (parameters.useCustomScenario)
     ApplyCustomScenario(study);

The last block if could merely be replaced with :

 study.scenarioRules.apply();

Remarks :

• Note that the if statement disappears : we plan to internalize it, as said in this discussion's presentation (ti's removed from the study as a data member).
• Note that these 3 tasks have nothing to do with running the simulation : they could be executed before running it, and clear a bit this large class. The planned work around scenario builder would be the opportunity to do it, even if we don't get a direct benefit from it.
• Scenario builder and class Study : as already said in the main discussion, we plan to keep the scenario builder as a data member of class Study : scenarioRules (too much work otherwise).
• Class Sets (class for set of rules, type of the above scenarioRules) : we do not plan to change this class for now, maybe will have to be internally adapted (remains to be seen)
• Scenario builder and GUI : GUI code will have to be adapted, but it should not be time consuming.