-
Notifications
You must be signed in to change notification settings - Fork 30
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
Changes from 17 commits
0a96da5
9ea0df2
6686d60
1c75d2b
a5bc8d8
3fcb4a7
2f09ba2
2e79678
3afaa92
26b7515
4bb1528
d045749
5d09ded
5770f22
0507887
d895250
49c25ba
7c4c0c4
2b066c2
fdfa326
03f4ca9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,180 @@ | ||
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, where in each archetypes one needs to add | ||
the new specification in the arcehtype header, then assign it and use it in the | ||
cpp file, and the developer must ensure the consistency in the variable naming and | ||
implementation across multiple archetypes. | ||
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_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_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 the snippet in the class header core: | ||
``#include toolkit/my_snippet.cycpp,h``. | ||
|
||
2. (optional) In the ``Archetype::EnterNotify()``, initialise the toolkit class member | ||
variables with variables. | ||
|
||
3. (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_snippet.cycpp.h``: | ||
.. highlight:: c | ||
cyclus::toolkit::Position coordinates(0,0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is correct, (and I like the versatility it provides)
(this is also why without inheritance has my (slight) preference.) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
class fun_archetype : public cyclus::facility{ | ||
public: | ||
[...] | ||
private: | ||
[...] | ||
#include "toolkit/my_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_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 | ||
class fun_archetype : public cyclus::facility, public Position { | ||
public: | ||
[...] | ||
private: | ||
[...] | ||
#include "toolkit/my_snippet.cycpp.h" | ||
} | ||
|
||
``my_archetype_example.cpp``: | ||
.. highlight:: c | ||
void fun_archetype::EnterNotify() { | ||
this.set_position(latitude, longitude); | ||
this.RecordPosition(this); | ||
[...] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still a long, awkward sentence.