Current delineation for Bucharest different from what is documented in the README and vignettes

[cforgaci]

@fnattino, as noticed in https://github.com/CityRiverSpaces/rcrisp/pull/370/changes#r3352247498, delineation is different from what we have documented in the README and in vignettes:

I have also checked the case of Warsaw, and the differences are considerable:

Any thoughts on what change in the package led to this? My guesses are: changes in network pre-processing or some change in corridor (or maybe valley) delineation. Let me know if you have any hints on how to diagnose this. It might be that the delineation remains as it is now, but I would like to understand what led to the change.

[cforgaci]

One thing that might help diagnose problems like this is a quick way to plot the river, the delineated valley, the network and the delineation. The plot.delineation method we are planning to implement should do that.

[cforgaci]

And beyond visual inspection (user-oriented), we should test this to avoid changes in the codebase that lead to considerable changes in the resulting delineation. I suggest:

• A delineation regression test, that is, checking against a known case within a certain tolerance.
• Property-based tests to catch obviously wrong output (we might have some of these already)

[fnattino]

Hi @cforgaci, this is indeed weird. What I suspect, however, is that the (main?) source of differences is not changes in the code, but changes in the input data. To check this, I went back to the commit at which the previous version of the figure was generated and rerun the code snippet in the README file - this produced the new figure you obtained (right panel above). On the contrary, using the example OSM data that we uploaded to Zenodo together with the code that is currently in the main branch produces a corridor that is identical to the older delineation (left panel above).

So maybe comparing the old input data from Zenodo vs the new input data recently downloaded from OSM can shed some light on why the delineation changed so drastically?

[fnattino]

I suggest:

• A delineation regression test, that is, checking against a known case within a certain tolerance.

This would be helpful, but considering all the issues we had in the past with tests on CI and CRAN with remotedatasets I would mark them as "to skip" anywhere but on local systems. I would also think it is more helpful to implement such regression test using the data "snapshot" on Zenodo, to make sure the code is actually sound (not sure whether it makes sense to have a test to verify that data did not change).

[cforgaci]

As seen in the screenshots below, we are currently getting a side-canal which has "Dâmbovița" in its name, but not as an exact match - it is called "Canal Argeș - Dâmbovița":

I think this was introduced in #176 when we started querying waterway: (with empty tag) instead of waterway:river to broaden the search where smaller streams would not be included.

In the case of Bucharest, it does not make sense to include the canal as it is an artificial waterway connecting Dambovita's catchment with the catchment of the neighbouring river Arges (possible on relatively flat terrain), which goes against the logic of the method.

As we cannot revert to waterway:river, I am considering introducing a logical parameter exact in match_osm_name() which is turned on when retrieving river centerlines. This might also be somehow related to #225. What do you think?

[fnattino]

As we cannot revert to waterway:river, I am considering introducing a logical parameter exact in match_osm_name() which is turned on when retrieving river centerlines. This might also be somehow related to #225. What do you think?

Mmh, If I recall correctly we had the exact match there at the very beginning of rcrisp, but had to drop that due to inconsistencies in the river names (e.g. "Rio Manzanares" and "Fiume Po"). What about using the solution implemented in the app (i.e. the solution suggested in #225)? From a very quick look the river geometry is fetched here: https://github.com/CityRiverSpaces/rcrisp-app/blob/2cbca7354c6159ca20ec64abf3a416a84dd2aa04/app.R#L134-L153 . Could we give it a try?