search

mperrinel / sst-core

SST Structural Simulation Toolkit Parallel Discrete Event Core and Services
http://www.sst-simulator.org
Other
0 stars 0 forks source link

Create Statistic object in the python File #8

Open mperrinel opened 4 years ago

mperrinel commented 4 years ago

Modify the C++/Python in the simulation to permit to the statistic objects to exist in the Python like Components and Subcomponents.

  statObj = comp.enableStatistic("sst.StatVtk")
  statObj.addParams(...)
mperrinel commented 4 years ago

@jjwilke .

I changed :

The idea in the pymodel_comp is to handle the Statistic as it is done for the SubComponent. To do that, I have created a new struct named PyStatistic that inherits from ComponentHolder:

struct PyStatistic : ComponentHolder {
    PyStatistic(ComponentPy_t *pobj, ComponentId_t id) : ComponentHolder(pobj,id) { }
    ~PyStatistic() {}
    int getSlot();
};

I decided to start with a Slot variable member, like it is done for the PySubComponent, but I am not sure that we want to attach a statistic to a component like we can attach a SubComponent to a component.

// pymodel_comp.h
ConfigComponent* getSubComp(const std::string& name, int slot_num);
ConfigStatistic* getStatistic(const std::string& name, int slot_num);
...

extern PyTypeObject PyModel_SubComponentType;
extern PyTypeObject PyModel_StatisticType;

Basically, I duplicated the code done for SubComponent to the Statistic.

For the moment, we only want to enable the addParam method for a Statistic from the python parameter file : 

// pymodel_comp.cc
static PyMethodDef statisticMethods[] = {
    {   "addParam",
        compAddParam, METH_VARARGS,
        "Adds a parameter(name, value)"},
    {   "addParams",
        compAddParams, METH_O,
        "Adds Multiple Parameters from a dict"}
   };

For the statistic creation, instead of modifying the enableStatistic method, I added a new compSetStatistic method (same logical as compSetSubComponent method). The idea is to create a statistic like that : 

// Python. file
stat = a.setStatistic("port%d"%p, "sst.StatVTK")

or without the slot/port parameter:

// Python. file
stat = a.setStatistic("sst.StatVTK")

I updated the pymodel.cc :

... 
PyModel_SubComponentType.tp_new = PyType_GenericNew;
PyModel_StatisticType.tp_new = PyType_GenericNew;
...
( PyType_Ready(&PyModel_SubComponentType) < 0 ) ||
( PyType_Ready(&PyModel_StatisticType) < 0 ) ||  // This line make crash the simulation
...
Py_INCREF(&PyModel_SubComponentType);
Py_INCREF(&PyModel_StatisticType);
...
PyModule_AddObject(module, "SubComponent", (PyObject*)&PyModel_SubComponentType);
PyModule_AddObject(module, "Statistic", (PyObject*)&PyModel_StatisticType);
...

There is currently a crash here : 

( PyType_Ready(&PyModel_StatisticType) < 0 ) ||  // This line makes crash the simulation

For the moment, I have absolutely no idea of the problem here since PyModel_StatisticType seems to be exactly defined like PyModel_SubComponentType is defined in the pymodel_comp.cc/.h files. Do I need to register the PyModel_StatisticType somewhere else ? This is the backtrace using lldb: 

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00007fff68118e52 libsystem_platform.dylib`_platform_strlen + 18
    frame #1: 0x00000001003f5047 Python`PyString_FromString + 23
    frame #2: 0x00000001003e6ca2 Python`PyDict_GetItemString + 18
    frame #3: 0x0000000100404a78 Python`PyType_Ready + 1080
    frame #4: 0x00000001000b7a14 sstsim.x`SST::Core::SSTPythonModelDefinition::initModel(this=0x000000010452ad60, script_file=<unavailable>, verbosity=<unavailable>, config=<unavailable>, argc=<unavailable>, argv=0x000000010452ac90) at pymodel.cc:852:12 [opt]
    frame #5: 0x00000001000b8192 sstsim.x`SST::Core::SSTPythonModelDefinition::SSTPythonModelDefinition(this=0x000000010452ad60, script_file="/Users/perrinel/Dev/sst/sst-core-test-component/basic.py", verbosity=1, configObj=<unavailable>) at pymodel.cc:949:5 [opt]
    frame #6: 0x000000010000156d sstsim.x`main(argc=<unavailable>, argv=<unavailable>) at main.cc:402:32 [opt]
    frame #7: 0x00007fff67f22cc9 libdyld.dylib`start + 1

And the frame 4:

frame #4: 0x00000001000b7a14 sstsim.x`SST::Core::SSTPythonModelDefinition::initModel(this=0x000000010452ad60, script_file=<unavailable>, verbosity=<unavailable>, config=<unavailable>, argc=<unavailable>, argv=0x000000010452ac90) at pymodel.cc:852:12 [opt]
   849           ( PyType_Ready(&PyModel_SubComponentType) < 0 ) ||
   850           ( PyType_Ready(&PyModel_LinkType) < 0 ) ||
   851           ( PyType_Ready(&PyModel_StatGroupType) < 0 ) ||
-> 852           ( PyType_Ready(&PyModel_StatisticType) < 0 ) ||
   853           ( PyType_Ready(&PyModel_StatOutputType) < 0 ) ||
   854           ( PyType_Ready(&ModuleLoaderType) < 0 ) ) {
   855          output->fatal(CALL_INFO, 1, "Error loading Python types.\n");

In addition, I have also updated the configGraph.h/.cc to add a new ConfigStatistic class that follows the model of the ConfigComponent class : 

// configGraph.h
class ConfigStatistic : public SST::Core::Serialization::serializable {
public:
    StatisticId_t id;                /*!< Unique ID of this component */
    ConfigGraph*  graph;            /*!< Graph that this component belongs to */
    std::string name;
    int slot_num;                    /*!< Slot number. */
    std::string type;
    Params params;
    ComponentId_t component;
    size_t outputID;
    UnitAlgebra outputFrequency;
...
  std::vector<ConfigComponent>  subComponents; /*!< List of subcomponents */
  std::vector<ConfigStatistic>  statistics; /*!< List of subcomponents */
...
    ConfigStatistic* addStatistic(StatisticId_t, const std::string& name, const std::string& type, int slot);
    ConfigStatistic* findStatistic(StatisticId_t);
    const ConfigStatistic* findStatistic(StatisticId_t) const;
    ConfigStatistic* findStatisticByName(const std::string& name);
...
    uint16_t                      nextSubID;         /*!< Next subID to use for children */
    uint16_t                      nextStatID;         /*!< Next subID to use for children */
...
    ComponentId_t getNextSubComponentID();
    StatisticId_t getNextStatisticID();
...
    const ConfigComponent* findSubComponent(ComponentId_t) const;
    ConfigComponent* findSubComponentByName(const std::string& name);
    ConfigStatistic* addStatistic(StatisticId_t, const std::string& name, const std::string& type, int slot);
    ConfigStatistic* findStatistic(StatisticId_t);
    const ConfigStatistic* findStatistic(StatisticId_t) const;
    ConfigStatistic* findStatisticByName(const std::string& name);
...
/** Map IDs to Components */
    typedef SparseVectorMap<ComponentId_t,ConfigComponent> ConfigComponentMap_t;
/** Map IDs to Statitics */
    typedef SparseVectorMap<StatisticId_t,ConfigStatistic> ConfigStatisticMap_t;
...
    ConfigComponentMap_t comps;
    ConfigStatisticMap_t stats;

In the ConfigGraph file, the idea is again to duplicate what is done for SubComponent, for the Statistic.

Thanks

jjwilke commented 4 years ago

Each component/subcomponent keeps an array called enabledStatistics. Probably we want this to be a ConfigStatistic object that gets stored in this array. The config statistic only needs to hold the ID, parameters, and name of the statistic.

Statistics need to have their own IDs. Later we will want to allow the same statistic to be used in multiple subcomponents (i.e. multiple ports sending to the same Statistic object).

mperrinel commented 4 years ago

@jjwilke I cleaned the ConfigStatistic to only keep the id, the name and the parameters. I fixed also the crash with the PyModelStatisticType. I decided to remove to PyStatistic class from the ComponentHolder inheritance because ;

  1. Component and subComponent are components. A statistic is not a component
  2. Statistic doesn’t need to have.a getSubComponent method neither a getStatistic method
  3. Statistic should have an id number of type StatisticId_t instead of type ComponentId_t
  4. Statistic should have a getStat method that returns a ConfigStatistic and not a getComp method that returns a ConfigComponent.
  5. Comparing a component with another one or with a subcomponent makes sense. Comparing two statistics can also makes sense. But compare a Component with a Statistic doesn’t make sense.

As a result, PyStatistic has been moved in some dedicated files : pymodel_stat.h/.cc

mperrinel commented 4 years ago

@jjwilke The current version on the branch 'Create Statistic Object' permits to do that:

a = sst.Component("a", "ctest.CtestComponent")

statA = a.setStatistic("traffic_intensity")
statA.addParams({"type":"sst.StatVTK"})

The statA python object knows the methods defined on the statisticMethods of the pymodel_stat file:

static PyMethodDef statisticMethods[] = {
    {   "addParam",
        statAddParam, METH_VARARGS,
        "Adds a parameter(name, value)"},
    {   "addParams",
        statAddParams, METH_O,
        "Adds Multiple Parameters from a dict"},
     {   nullptr, nullptr, 0, nullptr }
};

It knows basically only the addParam(s) methods.

On the configGraph.h, I have replaced :

std::vector<Statistics::StatisticInfo> enabledStatistics;

by :

std::vector<ConfigStatistic>  enabledStatistics;

As a consequence, I had also to replace in the componentInfo.h: 

typedef std::vector<Statistics::StatisticInfo>      statEnableList_t;

by : 

typedef std::vector<ConfigStatistic>      statEnableList_t;

That way, the enabled statistics is correctly checked and created when calling the statistics registerMultiStatistic from the component.

I'll try to work on the statistics hailstone number collection tomorrow to verify that all of that works.

Does these modifications are correct for you ?

mperrinel commented 4 years ago

@jjwilke I moved the registration of the statistic from the SubComponent (ActivePort) to the Component (CTestComponent). Since the hailstone Events is created on the ActivePort and not on the CTestComponent, I added a setTrafficIntensity method that permits to the Component to give a pointer on the MultiStatistic registered to the subComponent. That way, the StatVTK addData_impl method continues to be called each time that a hailStone event is created from an ActivePort. The difference is that there is only one StatVTK instance per Component (not SubComponent), then, providing the port as a templated parameter becomes necessary.

I think I need your suggestion/validation on all of that works. I'm starting to work on the #9.

jjwilke commented 4 years ago

The ConfigStatistic looks good. Those are the minimal set of methods we'll want for now. As we add more features, we may want more methods, but nothing needed now.

jjwilke commented 4 years ago

What was the reason for moving registration to the Component? Ideally we would keep the register statistic call on the subcomponent. We can discuss this on the Friday call.

mperrinel commented 4 years ago

@jjwilke I added the Component_Id into the ConfigStatistic class. In addition, I have also modified the getNextStatisticId method to follow exactly what it is done for Component and SubComponent. That way, I have a global uniq Id for every statistics even if they are created from different components: I have removed the STATISTIC_ID_MAK and I use directly the COMPONENT_ID_MASK. I think it is the correct way to do that because that permits to retrieve the Component parent id of a SubComponent Id. Since the creation of a Statistic Id follows exactly the same rules of a SubComponent, that makes sense to retrieve the parent Component id of a statistic using COMPONENT_ID_MASK method. I also have duplicated the SUBCOMPONENT_ID_CREATE into a new STATISTIC_ID_CREATE method. They do exactly the same thing but I was not very comfortable by creating a new statistic id with a method named : SUBCOMPONENT_ID_CREATE.

Moreover, I have verified that creating a Statistic using the old method : enableStatistic is still correct, but with some differences with the new way:

  1. enableStatistic doesn't create a StatisticInfo anymore (implicit creation done by emplace_back call of a vector) (which, for me, should be now DEPRECATED) but a ConfigStatisic.
  2. The ConfigStatistic is created using this constructor, meaning the Statistic_Id member is not set : 
    ConfigStatistic(const std::string& name) :
  name(name)
  { }

As a consequence, it is not possible to retrieve this ConfigStatistic using an Id. For the moment, it is not a problem because this ID is only used to find the statistic for adding parameters (Python side). I think it is possible to generate a Statistic ID and to assign it to the ConfigStatistic, but doing that means, that I have to modify the old way to create a Statistic, which is not very compliant with the next release.

If we are done with the StatisticCreation (at least for now) I'll merge the branch to develop and continue on the StatisticGroup.