Re: [GRIDRPC-WG] [saga-core-wg] Grid RPC data management & SAGA
Quoting [Ga?l Le Mahec] (Sep 22 2009):
But I have a question, to help me understand: so far I had the impression that rpc calls would, in the new model, also accept a mixture of the old style parameters and the new style data handles - is that the case?
Yes it is the case. We tried to preserve a maximum backward compatibility and in the ideal implementation the Grid RPC call should accept old style parameters as well as data handles.
If yes, then we would need to change the parameters for the rpc.call() method in a way that both are accepted.
A possible solution could be to modify the current saga::rpc::parameter class to be an abstract class which is "derived" into two different subclasses: the first one encapsulates a saga::buffer and is similar to the current saga::rpc::parameter and the second one implements the new data handle. But maybe there are other ways to proceed (using an interface ?) avoiding to change too deeply the current rpc::parameter class.
I think that this would be the cleanest solution. It may be useful to add some rudimentary means for inspection to the abstract class, like get_type() which would return an enum 'Buffer' or 'Handle' - what do you think? That would also simplify implementations which only support one type. Well, that would make the class concrete (i.e. not abstract), but so what? One could leave the constructor private, so that the user cannot accidentically create the base class itself... Just thinking out loud here... Best, Andre. -- Nothing is ever easy.
Le 22 sept. 09 à 12:50, Andre Merzky a écrit :
[...] A possible solution could be to modify the current saga::rpc::parameter class to be an abstract class which is "derived" into two different subclasses: the first one encapsulates a saga::buffer and is similar to the current saga::rpc::parameter and the second one implements the new data handle. But maybe there are other ways to proceed (using an interface ?) avoiding to change too deeply the current rpc::parameter class.
I think that this would be the cleanest solution.
It may be useful to add some rudimentary means for inspection to the abstract class, like get_type() which would return an enum 'Buffer' or 'Handle' - what do you think? That would also simplify implementations which only support one type.
Yes, it is a good idea. We can also maybe add a type for the containers. So we would have classes like that: package saga.rpc { ... enum parameter_type { Buffer = 0, Handle = 1, Container = 2 } class parameter : extends saga::object { /* private constructor. For SIDL definition, just do not define it. * For the implementation it should look like that: * CONSTRUCTOR(in parameter_type type, * out parameter obj); */ get_type(out parameter_type type); } class buffer_parameter : extends parameter { /* We use the same constructor that the previous saga::rpc::parameter * In the implementation it calls the constructor of the inherited class: * parameter(Buffer) */ CONSTRUCTOR (in array<byte> data = "", in int size = 0, in io_mode mode = In, out parameter obj); ... } class handle_parameter : extends parameter { /* In the implementation the constructor calls parameter(Handle) */ CONSTRUCTOR(in array<int> dim_sizes, in array<uri> input_uris, in array<uri> output_uris, in data_type type, in array<data_mode> data_modes out parameter obj); ... } ... } With these definitions, we have an uniform way to call a service regardless to the data type (buffer, handle or container). Moreover, the containers can also manage these different types.
Well, that would make the class concrete (i.e. not abstract), but so what? One could leave the constructor private, so that the user cannot accidentically create the base class itself...
Just thinking out loud here...
Indeed, I think this is a good way to prevent such a problem. Gaël.
Quoting [Ga?l Le Mahec] (Sep 23 2009):
Yes, it is a good idea. We can also maybe add a type for the containers.
So we would have classes like that:
Some suggested changes below:
package saga.rpc { ... enum parameter_type { Buffer = 0, Handle = 1, Container = 2 }
class parameter : extends saga::object { /* private constructor. For SIDL definition, just do not define it. * For the implementation it should look like that: * CONSTRUCTOR(in parameter_type type, * out parameter obj); */
get_type(out parameter_type type); }
class buffer_parameter : extends parameter
This one should then also inherit saga::buffer I guess.
{ /* We use the same constructor that the previous saga::rpc::parameter * In the implementation it calls the constructor of the inherited class: * parameter(Buffer) */ CONSTRUCTOR (in array<byte> data = "", in int size = 0, in io_mode mode = In, out parameter obj); ... }
class handle_parameter : extends parameter { /* In the implementation the constructor calls parameter(Handle) */ CONSTRUCTOR(in array<int> dim_sizes, in array<uri> input_uris, in array<uri> output_uris, in data_type type, in array<data_mode> data_modes out parameter obj); ... }
...
What would a container_parameter look like? I am not sure I understood that one, yet.
}
With these definitions, we have an uniform way to call a service regardless to the data type (buffer, handle or container). Moreover, the containers can also manage these different types.
Well, that would make the class concrete (i.e. not abstract), but so what? One could leave the constructor private, so that the user cannot accidentically create the base class itself...
Just thinking out loud here...
Indeed, I think this is a good way to prevent such a problem.
Gaël.
Nice, looks like things are converging :-)) Best, Andre. -- Nothing is ever easy.
Some suggested changes below:
[...]
class buffer_parameter : extends parameter
This one should then also inherit saga::buffer I guess.
For a C++ mapping, it is probably the easiest and cleanest way but if I remember well, SIDL does not allow multiple inheritance. So I avoided to use it. A solution is then to encapsulate a buffer object inside the buffer_parameter class.
[...]
What would a container_parameter look like? I am not sure I understood that one, yet.
Ooops, I forgot to give the container class. class container_parameter : extends handle_parameter { /* For a container, the dimension size is the size of the * container which is increasing each time we add a data. * And the data type is "Container". * * So, the constructor call its parent constructor like this: * saga::rpc::handle_parameter([0], input_uris, * output_uris, * Container, * data_modes); */ CONSTRUCTOR(in array<uri> input_uris, in array<uri> output_uris, in array<data_mode> data_modes, out container_parameter obj); // add a data into this data container set_data(in parameter data, in int rank); // get a data from this data container get_data(out parameter data, in int rank); // add a data at the end of this data container push_back(in parameter data); ... } A container is then a "special" handle_parameter which contains a set of parameters (accessible using an index value or push/pop methods). We defined it in the Grid RPC data management document to manage sets of data. (We have some use-cases where a service is called with a variable length set of input data and a variable length set of output). By using the "parameter" class as the type a "container_parameter" can manage, all the different data types can be stored in the container. Regards, Gaël.
Quoting [Ga?l Le Mahec] (Sep 25 2009):
Some suggested changes below:
class buffer_parameter : extends parameter
This one should then also inherit saga::buffer I guess.
For a C++ mapping, it is probably the easiest and cleanest way but if I remember well, SIDL does not allow multiple inheritance. So I avoided to use it.
Oh, SIDL does allow multiple inheritance - and, for good or for bad, we use it in a number of places already. Mostly for interfaces though - but at the end, that is what your base parameter class provides, right? For example, the rpc class in SAGA is: class rpc : implements saga::object implements saga::async implements saga::permissions
A solution is then to encapsulate a buffer object inside the buffer_parameter class.
[...]
What would a container_parameter look like? I am not sure I understood that one, yet.
Ooops, I forgot to give the container class.
class container_parameter : extends handle_parameter { /* For a container, the dimension size is the size of the * container which is increasing each time we add a data. * And the data type is "Container". * * So, the constructor call its parent constructor like this: * saga::rpc::handle_parameter([0], input_uris, * output_uris, * Container, * data_modes); */
CONSTRUCTOR(in array<uri> input_uris, in array<uri> output_uris, in array<data_mode> data_modes, out container_parameter obj);
// add a data into this data container set_data(in parameter data, in int rank);
This should be 'add_data()'? Or is rank the id of the data item to be set?
// get a data from this data container get_data(out parameter data, in int rank);
// add a data at the end of this data container push_back(in parameter data); ...
}
A container is then a "special" handle_parameter which contains a set of parameters (accessible using an index value or push/pop methods). We defined it in the Grid RPC data management document to manage sets of data. (We have some use-cases where a service is called with a variable length set of input data and a variable length set of output).
I am not sure I understand the purpose of the container class, really. The rpc-data draft I found on GridForge (http://forge.ogf.org/sf/go/doc15355) does not mention it, really - could you send me an update, please?
By using the "parameter" class as the type a "container_parameter" can manage, all the different data types can be stored in the container.
Why is that actually needed on API level? The rpc call is getting a list of parameters, right (IIRC, its a vararg in the original C binding.) For SAGA, its a vector. So, that vector already acts as a container for all parameters. What additional functionality would a parameter_container provide? Cheers, Andre.
Regards,
Gaël.
-- Nothing is ever easy.
participants (2)
-
Andre Merzky -
Gaël Le Mahec