Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CEP27: toolkit Snippet #297

Merged
merged 21 commits into from
Mar 27, 2021
Merged
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 187 additions & 0 deletions source/cep/cep27.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
CEP 27 - Toolkit Capabilities Injection into an Archetype
*********************************************************

:CEP: 27
:Title: Agent Toolkit Capabilities
:Last-Modified: 2019-10-28
:Author: Baptiste Mouginot <[email protected]>
:Status: Active
:Type: Standards Track
:Created: Baptiste Mouginot


Abstract
========

The |Cyclus| toolkit is designed to easily implement specific capabilities in
newly developed archetypes, such as a trading policy, commodity producers, etc.
To add characteristics to archetypes such as `Position` or `Metadata`, the
actual implementation method is very verbose. It relies on adding the new
specification in the arcehtype header, assigning it and use it in the cpp
bam241 marked this conversation as resolved.
Show resolved Hide resolved
file. The developers have to manually ensure the consistency in the variable naming and
bam241 marked this conversation as resolved.
Show resolved Hide resolved
implementation across multiple archetypes/files.
This CEP explains introduces the concept of snippets to simplify and maintain consistency
in the implementation and the usage, across multiple archetypes.


Toolkit Implementation
======================

Each |Cyclus| toolkit component will contain 3 different files:
- 2 for the definition of the feature C++ class (``cpp`` and header) that allows
the use of the capabilities, and optionally to register its values in the
output database,
- a snippet definition file used to simplify the implementation and ensure
consistency accross its integration in the different archetypes.

The snippet definition file will be included in the ``private`` section of the
archetype header as: ``#include toolkit/my_feature_snippet.cycpp.h``. (The use of the
``cycpp.h`` has been chosen to allow syntax highlighting and inform developers
that this is not a standard C++ header.)

The snippet file, will contain the declaration of all the variables required
to use the capabilities class:

- the definition of the capability class as a member variable.

- initialisation of the added variables.
bam241 marked this conversation as resolved.
Show resolved Hide resolved

- (optional) if the capability requires/allows variable input from users,
standard |Cyclus| member variable declaration with variable ``#pragma`` is
required. In addition, to the standard variable declaration and the
``#pragma`` the method also require a ``std::vector<int>
cycpp_shape_myvariable`` to be declared for each of the decorated variable
that are in the `toolkit/my_feature_snippet.cycpp.h` file. (``cycpp preprocessor`` is
not able at the time to add them automatically for included files.)


The main purpose of this include method would be to ensure consistency across
archetypes using the same toolkit capability requiring user input, avoiding 1
set of input syntax per archetypes for the same capability.

If the toolkit features needs the capabilities to write in the output database a
``RecordSnippet(Agent* agent)`` method will be implemented in the toolkit class to avoid
multiplication of implementation in the each archetype using the feature.


Archetypes Integration
======================

When the capability is integrated in an Archetype the following implementations
have to be done:

1. Include toolkit class header in in the class header:
``#include 'toolkit/my_feature.h'``.
gonuke marked this conversation as resolved.
Show resolved Hide resolved

2. Include the snippet in the class header core:
``#include toolkit/my_feature_snippet.cycpp,h``.

3. (optional) In the ``Archetype::EnterNotify()``, initialise the toolkit class member
variables with variables.

4. (optional) If required, call the ``RecordSnippet()`` method when necessary during the
Archetype operation.


Class member vs Inheritance
===========================

With inheritance of the capability class, one will need to also modify the
archetype class declaration in addition to simply including the snippet at the
right place.
This may also lead to confusion, as one can believe that the user input value
for variable are passed in the constructor of the class and might lead the
develop to skip the assignation of the value in the inherited class in the
``EnterNotify``...

Otherwise behavior would be very similar.

Example:
========


Without Inheritance:
--------------------
``toolkit/my_feature_snippet.cycpp.h``:
.. highlight:: c
cyclus::toolkit::Position coordinates(0,0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you remove only this line (not exposed in input), then the snippet can be used in both modes, and developer chooses name of member variable.

In fact, that might be better, because otherwise developer has to know name of member variable defined in the snippet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is correct, (and I like the versatility it provides)
I guess it is a balance between versatility and amount of work on the developer side:
if we only support without-inheritance everything should be includable in the snippet.
if we want to support both, developers have to add:

  • #include 'toolkit/my_toolkit.h
  • inheritance or feature element
  • #include toolkit/snippet.cycpp.h` in the class core.

(this is also why without inheritance has my (slight) preference.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the good thing with removing the variable it would allow both with and without inheritance

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even without inheritance, I'm not sure we want to assume that the member variable is always defined by the snippet... what if you want two positions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure but then what is the use of the snippet ? because some member variable are defined in the snippet (at least the one expose in the input.)

I don't see how to use the snippet with 2 positions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not be the best example of including more than one instance. In addition, there may be a need to initialize each instance which gets complicated again. Nevertheless, I think asking the developer to explicitly declare their member variable is not too much to ask.

We should open this up to additional discussion, e.g. on the mailing list.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think there is a benefit to having a single snippet file and then letting user choose the variable name in the non-inheritance case. We can include that in the example in the documentation.

Once we get that resolved, and clean up this CEP, I think I'm ready to approve/merge this and the follow-up PR's in the code repos.


#pragma cyclus var { \
"default": 0.0, \
"uilabel": "Geographical latitude in degrees as a double", \
"doc": "Latitude of the agent's geographical position. The value should " \
"be expressed in degrees as a double." }
double latitude = 0;
// required for compilation but not added by the cycpp preprocessor...
std::vector<int> cycpp_shape_latitude;

#pragma cyclus var { \
"default": 0.0, \
"uilabel": "Geographical longitude in degrees as a double", \
"doc": "Longitude of the agent's geographical position. The value should" \
"be expressed in degrees as a double." }
double longitude = 0;
// required for compilation but not added by the cycpp preprocessor...
std::vector<int> cycpp_shape_longitude;

``my_archetype_example.h``:
.. highlight:: c
gonuke marked this conversation as resolved.
Show resolved Hide resolved
#include 'toolkit/Position.h'

class fun_archetype : public cyclus::facility{
public:
[...]
private:
[...]
#include "toolkit/my_feature_snippet.cycpp.h"
}

``my_archetype_example.cpp``:
.. highlight:: c
void fun_archetype::EnterNotify() {
coordinates.set_position(latitude, longitude);
coordinates.RecordPosition(this);
[...]
}

With Inheritance:
-----------------
``toolkit/my_feature_snippet.cycpp.h``:
.. highlight:: c
#pragma cyclus var { \
"default": 0.0, \
"uilabel": "Geographical latitude in degrees as a double", \
"doc": "Latitude of the agent's geographical position. The value should " \
"be expressed in degrees as a double." }
double latitude = 0;
// required for compilation but not added by the cycpp preprocessor...
std::vector<int> cycpp_shape_latitude;

#pragma cyclus var { \
"default": 0.0, \
"uilabel": "Geographical longitude in degrees as a double", \
"doc": "Longitude of the agent's geographical position. The value should" \
"be expressed in degrees as a double." }
double longitude = 0;
// required for compilation but not added by the cycpp preprocessor...
std::vector<int> cycpp_shape_longitude;

``my_archetype_example.h``:
.. highlight:: c
#include 'toolkit/Position.h'

class fun_archetype : public cyclus::facility, public Position {
public:
[...]
private:
[...]
#include "toolkit/my_feature_snippet.cycpp.h"
}

``my_archetype_example.cpp``:
.. highlight:: c
void fun_archetype::EnterNotify() {
this.set_position(latitude, longitude);
this.RecordPosition(this);
[...]
}