[gdal-dev] RFC 48: Geographical networks support

[Dmitry Baryshnikov]

Hi Even,

10.07.2015 00:19, Even Rouault пишет:
> Hi Dmitry,
>
>> I have just finished the integration of GSoC2014 work into the
>> geographical network support in GDAL.
>> The major changes from original code:
>> 1) The base network class (GNMNetwork) is inherited form GDALDataset. So
>> any GDAL utility, that works with GDALDataset can work with GNMNetwork.
>> 2) Geographical network is a new type of data, additionally to the
>> DCAP_RASTER and DCAP_VECTOR, added DCAP_GNM
>> 3) I created the generic implementation of GDAL geographical network
>> (GNMGenericNetwork). The generic network have some features: support of
>> connection rules, support virtual vertices and edges, etc. The
>> GNMNetwork API can be changed in future as common functions to generic
>> network and some other networks (pgRouting, ArcGIS, OSRM, etc.) should
>> be moved to the GNMNetwork class.
>> 4) Two network drivers created for generic network: file based
>> datasource and db based datasource. The file network driver tested on
>> ESRI shapefile and db - on PostGIS. The another vector datasource which
>> are not applicable with this drivers should have their own drivers.
> I could probably figure out that myself by looking at the code, but what are
> the main difference between the generic file network driver and the db file
> network driver ? Both should use the generic OGR API, no ?
The difference come from the OGR driver specific. For example, database can
store strings more than 255 characters, but shape file - not (I need to 
store the
spatial reference somethere), also in file based sources the layer usually
connect with it's own datasource,  and database have a one datasource 
for all
layers. Also, in file based datasource the network is a sonamed folder, and
during deletion process the folder may be deleted if it empty (if user 
put to
this folder it's own files, the folder will not deleted). In case of 
database the
network is a database schema. So, the such differences force me to make 
the two
drivers. Also this drivers show the example of implementation the 
generic network
support.
>
>> 5) The main method of created and configured GNMNetwork is GetPath,
>> which return the temporary OGRLayer (similar to ExecuteSQL).
>> 6) By default the GNM is switched off. The ./configure --with-gnm should
>> be run to enable GNM. Mayne set it on by default?
> I think the decision depends mostly on how much we are confident that the API
> is stable. While there's not a non-generic driver implemented, there's
> potentially a risk (I think, but I've not a strong opinion on this.)
Ok, let leave it. Also I change nmake to not build GNM by default
>
>> 7) The python binding is implemented and used in tests.
>>
>> The RFC 48 in most cases is actual, except C API, which is not
>> implemented.  Do we need it?
> The idea is that the SWIG bindings only needs to know the C API, which provide
> some insulation from C++ ABI changes. So you could theoretically use the GDAL
> 2.0.0 Python bindings with a GDAL 2.1.0 shared object.
> But the main reason might be to avoid issues related to C++ ABI if not using
> the same compiler for the GDAL dll and building the Python bindings. Which can
> happen on Windows since the Python bindings require the same MSVC version than
> the one used to compile Python itself. When sticking to the C ABI, you can be
> safe. With the C++ ABI, some bad things could potentially happen.
Done
>
>> https://trac.osgeo.org/gdal/wiki/rfc48_geographical_networks_support
>>
>> Links:
> Perhaps do you have a git branch somewhere (just for the sake of easier
> applying) ?
Ok, I send the pull request.
>
>> - diff with binary test data -
>> https://drive.google.com/file/d/0BzlLlHyrgQHkNkktUmxYbWtjV3M/view?usp=shari
>> ng - diff without binary test data -
>> https://drive.google.com/file/d/0BzlLlHyrgQHkZ2lZN2wwTTVOQlE/view?usp=shari
>> ng - binary test data -
>> https://drive.google.com/file/d/0BzlLlHyrgQHkTTJjRFZNOEtfb3c/view?usp=shari
>> ng
>>
>> Code reviewing and testing are welcome. Also the some decisions in
>> implementation can be discussed.
>  From a random skimming, I'm wondering about the below :
>
> +%inline %{
> +  GNMNetworkShadow* CastToNetwork(GDALMajorObjectShadow* base) {
> +    return static_cast<GNMNetworkShadow*>(base);
> +  }
> +%}
> +
> +%inline %{
> +  GNMGenericNetworkShadow* CastToGenericNetwork(GDALMajorObjectShadow* base)
> {
> +    return static_cast<GNMGenericNetworkShadow*>(base);
> +  }
> +%}
> +
>
> Shouldn't that be a dynamic_cast ? Otherwise you'll manage to cast any
> GDALMajorObject into a GNMxxxx even when it is not valid. You could probably
> test that by trying to cast a TIFF dataset or something completely irrelevant.
The compiler didn't let me dynamic cast from void* to void* (as the
GDALMajorObjectShadow* is typedef void* and GNMNetworkShadow*,
GNMGenericNetworkShadow* also void). Any idea how to solve this?
>> The GDAL 2.0 provide ability to create datasets combine vector and
>> raster data (drivers support both types).
>> In case of PostGIS we can have the driver combined vector, raster and
>> networks (pgRouting). Maybe someone give an idea how it can be done.
>
> Currently, from what I see from the generic network implementation, the whole
> graph is loaded from storage into memory at Open() time
> (GNMFileNetwork::Open() calling LoadGraphLayer()). Right ?  Wondering how much
> a limitation it can be in practice. Couldn't the loading of the graph be
> defered up to the point it is really needed, so that Open() is kept reasonably
> fast ? I don't see any try catch around insertions into STL containers, so I
> guess that things might blow up if a too big graph is loaded.
Yes, there is a note in graph description (doxygen comments) that this
implementation are limited and may be potential problems with big graphs.
The GNMGraph class is a subject of future improvements. But lazy loading 
of the
graph data make sense. Done it.
>
> Even
>

Best regards,
     Dmitry