[GRASS-dev] iostream issues (formerly r.viewshed ready for testing)

GRASS GIS GRASS GIS developer
1

Hi Paul,

So, I think I know what is causing the problems with r.terraflow and r.viewshed, and I solved it with r.terraflow. r.terraflow needed the change in line you outlined earlier (changing IOSTREAMDEP = $(ARCH_LIBDIR)/$(LIB_PREFIX)$(IOSTREAM_LIBNAME)$(LIB_SUFFIX) to IOSTREAMDEP = (ARCH_LIBDIR)/$(LIB_PREFIX)$(IOSTREAM_LIBNAME)$(STLIB_SUFFIX) in Grass.make.in) in both Grass.make.in and Grass.make. So, r.terraflow is all set (at least for me).

r.viewshed is a completely different issue. It seems like it isn’t able to see iostream at all - if I remove the include line for iostream, I get the exact same errors. I think this is probably because of the unusual makefile r.viewshed has. We weren’t able to get a makefile like r.terraflow to work with r.viewshed, so we more or less rewrote everything. I’ll try to mess around with it, and make it use more standard grass makefiles rather than rewriting them, but I’m not very familiar with how GRASS makes things, so I may not have much luck. If you or anyone else could take a look at the makefile, and try to make it more standard, I would appreciate it.

I’ll keep you posted on any updates,
-Will

On Mon, Aug 4, 2008 at 8:26 PM, Glynn Clements <glynn@gclements.plus.com> wrote:

Paul Kelly wrote:

So I got the trunk version of GRASS, and compiled it, but iostream and
r.terraflow gave me errors. As the instructions say, I tackled the iostream
error first, and I can’t figure out what’s wrong with it. Here is what I
get if I call make in grass_trunk/lib/iostream

cc -dynamiclib -compatibility_version 7.0 -current_version 7.0 -install_name
^^
GRASS is trying to use the C compiler to create the shared library - this
worked on Linux so I didn’t notice it but obviously that must have been
just luck and it seems not to work on OS X. This clearly is something I
need to look at and try to fix - none of the other libraries contain C++
code so this hasn’t come up before.

Another issue with making IOStream a dynamic library is that
Shlib.make only adds $(SHLIB_CFLAGS) to CFLAGS, not to CXXFLAGS.

On Linux, this causes the code to be compiled without -fPIC, resulting
in:

warning: creating a DT_TEXTREL in object.

Normally, this is just an efficiency issue (the library won’t actually
be shared; each process will have a separate copy), but it won’t work
on systems running SELinux (to perform relocation, the code segment
has to be modified, and SELinux won’t let it regain execute permission
after it has been modified).

Also, the effect will vary between platforms. The Darwin case in
SC_CONFIG_FLAGS has:

SHLIB_CFLAGS=“-fno-common”

ISTR that this may be a necessity for building dynamic libraries on
that platform.

I’ll commit a fix for this as soon as I have tested it.

–
Glynn Clements <glynn@gclements.plus.com>

–
-Will

2

On Tue, 5 Aug 2008, Will wrote:

r.viewshed is a completely different issue. It seems like it isn't able to
see iostream at all - if I remove the include line for iostream, I get the
exact same errors. I think this is probably because of the unusual makefile
r.viewshed has. We weren't able to get a makefile like r.terraflow to work
with r.viewshed, so we more or less rewrote everything. I'll try to mess
around with it, and make it use more standard grass makefiles rather than
rewriting them, but I'm not very familiar with how GRASS makes things, so I
may not have much luck. If you or anyone else could take a look at the
makefile, and try to make it more standard, I would appreciate it.

Hi Will,
Did you use the updated tarball for r.viewshed that I sent you, with the Makefile adjusted and the include paths for the iostream includes changed to match the new location? If yes, and you are getting errors different from the ones I included in the previous mail, please post the output you are getting so we can diagnose.

thanks,

Paul

3

Hi Paul,

Yes, I am using the updated tarball that you sent me, and I’m getting the same errors, regardless of if ami.h (the header for iostream) is included in distribute.cc or not.

-Will

On Tue, Aug 5, 2008 at 11:37 AM, Paul Kelly <paul-grass@stjohnspoint.co.uk> wrote:

On Tue, 5 Aug 2008, Will wrote:

r.viewshed is a completely different issue. It seems like it isn’t able to
see iostream at all - if I remove the include line for iostream, I get the
exact same errors. I think this is probably because of the unusual makefile
r.viewshed has. We weren’t able to get a makefile like r.terraflow to work
with r.viewshed, so we more or less rewrote everything. I’ll try to mess
around with it, and make it use more standard grass makefiles rather than
rewriting them, but I’m not very familiar with how GRASS makes things, so I
may not have much luck. If you or anyone else could take a look at the
makefile, and try to make it more standard, I would appreciate it.

Hi Will,
Did you use the updated tarball for r.viewshed that I sent you, with the Makefile adjusted and the include paths for the iostream includes changed to match the new location? If yes, and you are getting errors different from the ones I included in the previous mail, please post the output you are getting so we can diagnose.

thanks,

Paul

4

On Tue, 5 Aug 2008, Will wrote:

Hi Paul,

Yes, I am using the updated tarball that you sent me, and I'm getting the
same errors, regardless of if ami.h (the header for iostream) is included in
distribute.cc or not.

That seems to just be a coincidence. If I copy the ami_stream.h from the version of iostream that you included with r.viewshed into include/iostream (and run make in the top level directory so that this gets copied into the correct location for compilation), the compilation of r.viewshed gets a lot further, albeit with a lot of warnings.

Basically the issue seems to be that the version of iostream that you have been working with is quite different from the version that r.terraflow uses. In particular, the licence statement is different (includes Duke University advertising clause) and there is a comment
  * PEARL upgrades: Rajiv Wickremesinghe 2004, 2005

I don't know what PEARL is, but other than that do you know (or can find out) are the updates by Rajiv Wickremesinghe significant to the functionality, and should they be included in GRASS to work with r.terraflow as well? I notice some comments around the code initialled RW which suggest there are a lot of little changes all over the place.

I guess what we need to find out is how important these are, and should they be merged into GRASS. Or, a simpler solution would be can you make r.viewshed work with the version of iostream in GRASS. The alternative is having separate versions of the iostream library for the two modules which is really ugly IMHO.

Good luck,

Paul

5

Hi Paul,

PEARL is off (not defined) when compiling, so it is not significant. Two libraries is definitely ugly. We'll try to get r.viewshed to work with the iostream version in GRASS.

Until then, thanks for all the help.

-Laura

On Aug 5, 2008, at 12:45 PM, Paul Kelly wrote:

On Tue, 5 Aug 2008, Will wrote:

Hi Paul,

Yes, I am using the updated tarball that you sent me, and I'm getting the
same errors, regardless of if ami.h (the header for iostream) is included in
distribute.cc or not.

That seems to just be a coincidence. If I copy the ami_stream.h from the version of iostream that you included with r.viewshed into include/iostream (and run make in the top level directory so that this gets copied into the correct location for compilation), the compilation of r.viewshed gets a lot further, albeit with a lot of warnings.

Basically the issue seems to be that the version of iostream that you have been working with is quite different from the version that r.terraflow uses. In particular, the licence statement is different (includes Duke University advertising clause) and there is a comment
* PEARL upgrades: Rajiv Wickremesinghe 2004, 2005

I don't know what PEARL is, but other than that do you know (or can find out) are the updates by Rajiv Wickremesinghe significant to the functionality, and should they be included in GRASS to work with r.terraflow as well? I notice some comments around the code initialled RW which suggest there are a lot of little changes all over the place.

I guess what we need to find out is how important these are, and should they be merged into GRASS. Or, a simpler solution would be can you make r.viewshed work with the version of iostream in GRASS. The alternative is having separate versions of the iostream library for the two modules which is really ugly IMHO.

Good luck,

Paul

6

Will wrote:

So, I think I know what is causing the problems with r.terraflow and
r.viewshed, and I solved it with r.terraflow. r.terraflow needed the change
in line you outlined earlier (changing IOSTREAMDEP =
$(ARCH_LIBDIR)/$(LIB_PREFIX)$(IOSTREAM_LIBNAME)$(LIB_SUFFIX) to IOSTREAMDEP
= (ARCH_LIBDIR)/$(LIB_PREFIX)$(IOSTREAM_LIBNAME)$(STLIB_SUFFIX) in
Grass.make.in) in both Grass.make.in and Grass.make. So, r.terraflow is all
set (at least for me).

r.viewshed is a completely different issue. It seems like it isn't able to
see iostream at all - if I remove the include line for iostream, I get the
exact same errors. I think this is probably because of the unusual makefile
r.viewshed has. We weren't able to get a makefile like r.terraflow to work
with r.viewshed, so we more or less rewrote everything. I'll try to mess
around with it, and make it use more standard grass makefiles rather than
rewriting them, but I'm not very familiar with how GRASS makes things, so I
may not have much luck. If you or anyone else could take a look at the
makefile, and try to make it more standard, I would appreciate it.

I think that the #include directives within the IOStream headers may
need to be changed, e.g.:

- #include "ami_config.h"
+ #include <grass/iostream/ami_config.h>

I'm not sure that you can rely upon the preprocessor looking in the
directory containing the header file when the header name has
directory components.

--
Glynn Clements <glynn@gclements.plus.com>

7

Viewshed does not immediately compile with the iostream library in grass. Over the last years there were a few changes to it, but were never merged them into the grass trunk because they did not seem important enough at the time. Now that both viewshed and terraflow use the same library, I don't think that it makes sense to try to make viewshed compile with an older version; instead, I would rather update the library. I looked over the grass svn logs and the only significant changes to iostream that I could see are the following:

1. #ifdef __MINGW32__ (windows compatibility)
2. char* replaced with std::string (Andy Danner's patch a few weeks ago)
(is there anything else?)

I have merged these changes into my most recent version of iostream, which I am attaching; it is split into headers (include_iostream.tar), and cc files (lib_iostream.tar). If you place them into grass_trunk/include/iostream/ and grass_trunk/lib/iostream/ respectively, the library, terraflow and viewshed all compile and run (macosx 10.4).

This is a start to get r.viewshed to compile and run and get some feedback. If the general opinion is to keep the library unchanged, I guess we'll have to modify viewshed, which is doable, though not particularly exciting.

-Laura

(attachments)

lib_iostream.tar.gz (4.55 KB)
include_iostream.tar.gz (44.6 KB)

8

On Tue, 5 Aug 2008, Laura Toma wrote:

Viewshed does not immediately compile with the iostream library in grass. Over the last years there were a few changes to it, but were never merged them into the grass trunk because they did not seem important enough at the time. Now that both viewshed and terraflow use the same library, I don't think that it makes sense to try to make viewshed compile with an older version; instead, I would rather update the library.

Hello Laura,
That sounds like a good plan and what I expected would be the best solution. If there are no unintentional revertions it should be fine to apply since it is only used by r.terraflow currently anyway. I'll be offline until tomorrow evening (European time): I'll look at it then if noone else gets to it before me.

Paul

9

Hi Laura,

On Tue, 5 Aug 2008, Laura Toma wrote:

I have merged these changes into my most recent version of iostream, which I am attaching; it is split into headers (include_iostream.tar), and cc files (lib_iostream.tar). If you place them into grass_trunk/include/iostream/ and grass_trunk/lib/iostream/ respectively, the library, terraflow and viewshed all compile and run (macosx 10.4).

That all seems fine. I've committed the updates to SVN trunk and also merged them back into develbranch_6 (i.e. 6.4-svn).

This is a start to get r.viewshed to compile and run and get some feedback.

Will, do you want to commit it to grass-addons now? I've created a directory under raster/ for it to go. If you haven't got your OSGeo id and grass-addons access sorted out yet I can commit it for you if you want and then you could make further updates directly in the SVN. Let me know.

One issue I noticed was with viewshed.cc including rtimer.h - I'm not sure if this is supposed to be the rtimer.h in the iostream lib or the rtimer.h in the r.viewshed source (maybe it could be renamed to reduce confusion?). If it's meant to be including the iostream version, you can change the include line to
#include <grass/iostream/rtimer.h>
to pick it up. Or if it's meant to be the one in the r.viewshed directory,
#include "rtimer.h"
should get it (the quotes rather than angle brackets indicate to look in the current directory for include files before searching the default include directories).

Paul

10

Hi Paul,

There should be no rtimer in viewshed, only in the library. I was planning to upload the most recent version as soon as I get access to the svn.

-Laura

On Aug 7, 2008, at 7:00 PM, Paul Kelly wrote:

Hi Laura,

On Tue, 5 Aug 2008, Laura Toma wrote:

I have merged these changes into my most recent version of iostream, which I am attaching; it is split into headers (include_iostream.tar), and cc files (lib_iostream.tar). If you place them into grass_trunk/include/iostream/ and grass_trunk/lib/iostream/ respectively, the library, terraflow and viewshed all compile and run (macosx 10.4).

That all seems fine. I've committed the updates to SVN trunk and also merged them back into develbranch_6 (i.e. 6.4-svn).

This is a start to get r.viewshed to compile and run and get some feedback.

Will, do you want to commit it to grass-addons now? I've created a directory under raster/ for it to go. If you haven't got your OSGeo id and grass-addons access sorted out yet I can commit it for you if you want and then you could make further updates directly in the SVN. Let me know.

One issue I noticed was with viewshed.cc including rtimer.h - I'm not sure if this is supposed to be the rtimer.h in the iostream lib or the rtimer.h in the r.viewshed source (maybe it could be renamed to reduce confusion?). If it's meant to be including the iostream version, you can change the include line to
#include <grass/iostream/rtimer.h>
to pick it up. Or if it's meant to be the one in the r.viewshed directory,
#include "rtimer.h"
should get it (the quotes rather than angle brackets indicate to look in the current directory for include files before searching the default include directories).

Paul