Opened 11 years ago

Closed 10 years ago

#2024 closed defect (fixed)

r.li.setup generates incomplete r.li conf file

Reported by: matmar Owned by: rashadkm
Priority: normal Milestone: 6.4.4
Component: Raster Version: svn-releasebranch64
Keywords: r.li.setup Cc: grass-dev@…, lucadelu
CPU: x86-64 Platform: Linux

Description

I've noticed a difference between the configuration file I generated 3 weeks ago using vector map to overlay with the one I've just generated (same procedure, again using vector map to overlay). In detail, the second seems almost empty, without any specified regions. I've upgraded grass a couple of times between the two configuration files. I've attached the two files (old and new).

Thanks, Matteo

Attachments (4)

conf_file_old​ (875 bytes ) - added by matmar 11 years ago.
conf_file_new​ (22 bytes ) - added by matmar 11 years ago.
sample_area_vector_bash.diff​ (413 bytes ) - added by rashadkm 10 years ago.
change the interpreter
r_li_bug_2024.diff​ (15.9 KB ) - added by rashadkm 10 years ago.
r.li.* fixes

Download all attachments as: .zip

Change History (37)

by matmar, 11 years ago

Attachment: conf_file_old​ added

by matmar, 11 years ago

Attachment: conf_file_new​ added

comment:1 by hamish, 11 years ago

Hi,

was it strictly with 6.4.svn, or did you mix in devbr6/6.5.svn at any point?

I've been doing a bit of cleanup in r.li.setup in 6.5svn, but it's too late in the 6.4.3 release schedule to backport any of it. I did backport to 6.4 one bugfix though, but it should have been obviously broken before, and only to do with the interactive digitizing, nothing to do with vector maps. I would not be too surprised to learn there was a typo in the 6.5svn changes still, they are only lightly tested so far.

6.4 changes to r.li: (click on the changeset [number] to view the difference)

​https://trac.osgeo.org/grass/log/grass/branches/releasebranch_6_4/raster/r.li

are you sure the computational region was set the same for the second try? no MASK or anything getting in the way?

?, Hamish

comment:2 by matmar, 11 years ago

Summary: r.li.setup → r.li.setup generates incomplete r.li conf file

comment:3 by hamish, 11 years ago

Hi,

could you post the exact steps to reproduce this? (preferably with one of the standard sample datasets) r.li.setup has many options..

thanks, Hamish

comment:4 by matmar, 10 years ago

Hi Hamish, below you find a minimum repeatable example that I've done using the North Carolina GRASS mapset.

Step-by-step bug report for r.li.setup.

GRASS 6.4.3svn (nc_spm_08):~ > g.version -g version=6.4.3svn revision=57227 date=2013

### r.li.setup test ## select three random polygons from boundary_county@PERMANENT as county_test ## Download North Carolina land use from: ​ftp://ftp.nconemap.com/outgoing/raster/past_imagery/state/landcover_1996/ r.in.gdal input='/home/matteo/lc96ras.img' output=lc96ras ### Region specification GRASS 6.4.3svn (nc_spm_08):~ > g.region rast=lc96ras -p projection: 99 (Lambert Conformal Conic) zone: 0 datum: nad83 ellipsoid: a=6378137 es=0.006694380022900787 north: 353129.25 south: -11385.75 west: 43706.5031 east: 959126.5031 nsres: 28.5 ewres: 28.5 rows: 12790 cols: 32120 cells: 410814800 ## Configuration steps in r.li.setup r.li.setup configuration file=test Raster map to use to select areas=lc96ras@landsat Vector map to Overlay=county_test@landsat Whole maplayer Select areas from the overlayed vector map ## Look at the generated (empty) setup file GRASS 6.4.3svn (nc_spm_08):~ > cat /home/matteo/.r.li/history/test SAMPLINGFRAME 0|0|1|1 ## try to run r.li.edgedensity with the generated setup file GRASS 6.4.3svn (nc_spm_08):~ > r.li.edgedensity map=lc96ras@landsat conf=test output=edge_test ERROR: Program error, worker() toReceive.type=-1572453528 ERROR: Program error, worker() toReceive.type=-1572453528 ERROR: Program error, worker() toReceive.type=-1572453528 ERROR: Program error, worker() toReceive.type=-1572453528 ERROR: Program error, worker() toReceive.type=-1572453528 ERROR: Program error, worker() toReceive.type=-1572453528 ERROR: Program error, worker() toReceive.type=-1572453528 ERROR: Program error, worker() toReceive.type=-1572453528 ERROR: Program error, worker() toReceive.type=-1572453528 ERROR: Program error, worker() toReceive.type=-1572453528 Segmentation fault (core dumped)

Best, Matteo

comment:5 by rashadkm, 10 years ago

Hi Matteo,

This seems to be a problem with the dash vs bash. Some linux distro (Debian, Ubuntu etc..) uses dash as the default shell for some reasons[1]. This means /bin/sh is symlinked to /bin/dash instead of bash shell. The two are different in some aspects like loops, comparison ops etc. Most of the bash shell runs ok with dash but some others won't. changing the default shell by changing #!/bin/sh to !/bin/bash fixes the problem.

[1] ​https://wiki.ubuntu.com/DashAsBinSh

by rashadkm, 10 years ago

Attachment: sample_area_vector_bash.diff​ added

change the interpreter

comment:6 by rashadkm, 10 years ago

Owner: changed from grass-dev@… to rashadkm
Status: new → assigned

Hi Matteo,

Could you try to apply the diff (r_li_bug_2024.diff) to r.li and try creating conf file and running r.li.edgedensity.

Here is my output: 

r.li.edgedensity map=lc96ras@landsat conf=lsat_ol output=edge_out

gives this in ~/r.li/output/edge_out (i had selected 3 random polygon from boundary_county of nc_spm_08 dataset. 

RESULT 2|5449.735450
RESULT 1|9355.742297
RESULT 3|2478.632479

I had also tested r.li.patchdensity but with moving window configuration.

by rashadkm, 10 years ago

Attachment: r_li_bug_2024.diff​ added

r.li.* fixes

in reply to:  6 ; comment:7 by neteler, 10 years ago

I have applied the proposed patch in r58848, r58849, r58850 with two tiny corrections (msg typo) for G6.4, 6.5 and G7.

Hopefully we are now on the way to stabilize the r.li suite.

in reply to:  7 comment:8 by neteler, 10 years ago

Reverted r58850 (G7) since it fails compilation.

comment:9 by hamish, 10 years ago

Cc: grass-dev@… added

Hi,

(housekeeping: if you take ownership of a ticket be sure to add the grass-dev list back to the cc field; please follow grass_indent.sh conventions)

what does the 'raster_' variable do?

as per comment:1, & to avoid double-handling, note that I've made many cleanups and fixes for r.li (particularly r.li.setup and its scripts) in devbr6 which (erhm, sorry) I have not yet committed or backported to relbr64. The job is still a construction site; I haven't forgotten about or abandoned it :). fwiw, I notice a couple minor places in the C code where I need to sync with trunk too. So more fixes to come, watch this space.

For now I would suggest to focus any r.li testing for 6.x in the 6.5 development branch. I will continue work on bugfixes for the tcl/bash version of r.li.setup there and backport bits to the stable branch once they are tested.

Note newly introduced bashism in sample_area_vector.sh (CAT_LIST[] array) in the latest patch. We need to be working the other way to remove them. :) Otherwise thanks, it all helps.

regards and thanks for your continued patience, Hamish

comment:10 by hamish, 10 years ago

(to avoid merge-hell, I would suggest to wait until I've committed my changes to r.li* before continuing other cleanups)

comment:11 by rashadkm, 10 years ago

Hi Hamish,

I had some more fixes in some r.li. modules which are still not working, unlike its other friends before. I will do this on devel branch for grass. IIUC, is there any changes need to reverted?. Because things are almost running fine and I have no idea about you uncommitted changes. For r.li.setup I dont have any more changes to push or didnt see any problems reported. BTW, I would like to add automatic naming for sampling area (overlay vector option) rather than user input which I think its better to be in devel branch.

Regarding newly introduced bashism, do you have a fix for it? or should i do another patch?

raster_ variable: when r.li.setup writes configuration file it write FQN of the raster map which it should be and when i was testing with some copies of same map in different mapsets. it reports error like "conf can only be used with map@landsat not with map. To avoid confusion and provide a better error results i made the FQN of the input raster map in raster_ variable.

comment:12 by neteler, 10 years ago

The fixes attached to this ticket are in SVN.

Furthermore I have now synch'ed the GRASS 6 branches incl HTML pages. Likewise the G7 HTML pages.

The fixes attached to this ticket are not yet forward-ported to G7.

comment:13 by hamish, 10 years ago

Hi,

I still have a lot of changes to commit to the r.li.setup shell scripts in devbr6 (the patches are over 650 lines long), so I would ask a little time to merge those before making any more changes and diverging the code. I will sync with your patch and look+learn closely at it. I don't have a fix for the bashism, but I haven't looked to closely at it or if it will conflict with my changes. I'll sync my changes with what's in svn now, no need to revert if it's working; my fault for leaving it so long. I probably shouldn't spend any more time on this today, but I'll look at syncing the new patch with trunk as a top priority if no one else has done it first.

(how's the status of wx-r.li.setup in trunk? is the tcl/shell version in trunk still needed as a reference?)

fwiw I had not backported the num_workers code before because it wasn't well tested or what I consider properly finished (see comment:1), but I guess now it is done we'll have to do some more serious testing. i.e. I wouldn't be surprised if a little more work will be needed, but the situation should on the whole be better than before, so we're still moving forward..

for some sort of coordinated plan, I would suggest to focus on trunk to work on r.li C modules, and devbr6 to work on the r.li.setup tcl/bash code. I would suggest any new major development to happen in trunk first.

there seemed to be common issues with the C modules around free()ing memory, I see you've come across the same. All these modules need a good run through Valgrind.

wrt the 'raster_' variable, my main point is please use self-documenting variable names. so -> raster_fqn if that's what it is. if there is code which is not handling map@mapset or without @mapset in the usual grass way (e.g. the imagery library long had issues with that), we should open new tickets for that and fix it too instead of cluttering the code by patching around 3rd party bugs.

thanks both Rashad & Markus for taking it up, r.li has been waiting for this work for some time!

Hamish

comment:14 by rashadkm, 10 years ago

Hi Hamish,

Fine. Take your time to merge those changes and let me know. I will not touch r.li.setup in grass6 until then. I will work on the C modules now. I think its better to use grass devbranch rather than grass70. ofcourse i can provide a corresponding patch for it simultaneously. The reason is r.li.setup tcl is lacking in grass70 and it would be better to test for others also if there is everything working. what do you think?

g.gui.rlisetup is still in a state with missing functionality. I had some patches for it and sent to Luca for reviewing it and he will be updating g.gui.rlisetup in vienna code sprint (putting up missing parts).

I will provide a new patch with your suggestion for variable naming.

in reply to:  14 ; comment:15 by wenzeslaus, 10 years ago

Replying to rashadkm:

Hi Hamish,

I think its better to use grass devbranch rather than grass70. ofcourse i can provide a corresponding patch for it simultaneously. The reason is r.li.setup tcl is lacking in grass70 and it would be better to test for others also if there is everything working. what do you think?

Providing patches simultaneously for develbrach 6 and trunk is better, in my opinion. If you just patch develbrach 6, the changes must be forward ported one day and it might be much harder to do it (for you or somebody else) just because of loosing track what is in code.

g.gui.rlisetup is still in a state with missing functionality. I had some patches for it and sent to Luca for reviewing it and he will be updating g.gui.rlisetup in vienna code sprint (putting up missing parts).

Tck/Tk will be dropped, so all the work done in it will be lost, thus it is much better to focus on improving g.gui.rlisetup (rather than improving Tck/Tk setup).

comment:16 by hamish, 10 years ago

Cc: lucadelu added

Hi,

Changeset 58878 Timestamp:

02/04/14 07:09:15 (5 hours ago)

Author:

lucadelu

Message:

r.li.setup: removed duplicate cat for same features 

Index: grass/branches/releasebranch_6_4/raster/r.li/r.li.setup/sample_area_vector.sh
===================================================================
--- a/grass/branches/releasebranch_6_4/raster/r.li/r.li.setup/sample_area_vector.sh
+++ b/grass/branches/releasebranch_6_4/raster/r.li/r.li.setup/sample_area_vector.sh
@@ -2,3 +2,3 @@
-#!/bin/bash
+#!/bin/bash -x
 #
 # This program is free software under the GPL (>=v2)
@@ -57,5 +57,5 @@
 
 #using v.category instead of v.build with cdump
-v.category input=$GIS_OPT_vector option=print > $TMP.cat
+v.category input=$GIS_OPT_vector option=print | sort | uniq > $TMP.cat
 
 #get input vector name

I'm copying this patch here as it will surely be dropped and forgotten otherwise.

some general requests to all,

Please avoid to commit changes to G6 tcl/shell r.li.setup until I have finished my merge (collect patches here instead), and please commit to the dev branch first then backport to the stable branch only once the change is fully tested and you are willing to sign it off. It's hard to keep track of what work needs to be done and use the automated branch-merge tools otherwise (and without those maintaining multiple branches is a real pain).

Also, it is best to always "quote" shell script $VARIABLES, especially when they contain filenames which can often have spaces in their path.

thanks, Hamish

in reply to:  15 comment:17 by rashadkm, 10 years ago

Replying to wenzeslaus:

I agree tcl/tk is replaced by wxgui. But in this case only working solution to generate a conf file is using grass64 r.li.setup script. The new addition to tcltk could also be added in g.gui.rlisetup. As the work is not finished there, I thought to add this modification in grass65 (devel branch). Also r.li.setup with x mon was fast to render large raster data which i was used in testing and rendering large maps on wxgui is still slow. Think of 100 sample region drawn on a 365MB data. Each time wx monitor will redraw the map (which it must be) and user zoom to an sampling region. So modification to trunk is the correct way but in this case no options.

Replying to rashadkm:

Hi Hamish,

I think its better to use grass devbranch rather than grass70. ofcourse i can provide a corresponding patch for it simultaneously. The reason is r.li.setup tcl is lacking in grass70 and it would be better to test for others also if there is everything working. what do you think?

Providing patches simultaneously for develbrach 6 and trunk is better, in my opinion. If you just patch develbrach 6, the changes must be forward ported one day and it might be much harder to do it (for you or somebody else) just because of loosing track what is in code.

g.gui.rlisetup is still in a state with missing functionality. I had some patches for it and sent to Luca for reviewing it and he will be updating g.gui.rlisetup in vienna code sprint (putting up missing parts).

Tck/Tk will be dropped, so all the work done in it will be lost, thus it is much better to focus on improving g.gui.rlisetup (rather than improving Tck/Tk setup).

in reply to:  16 comment:18 by neteler, 10 years ago

Replying to hamish:

r58878

...

I'm copying this patch here as it will surely be dropped and forgotten otherwise.

I have fixed that and also inverted the -x issue.

some general requests to all,

Please avoid to commit changes to G6 tcl/shell r.li.setup until I have finished my merge (collect patches here instead),

Do you have an estimated time for this? The last activity was 7 months ago and we (FEM and more users) need a working version NOW. That's why we have started our bugfixing activity.

and please commit to the dev branch first then backport to the stable branch only once the change is fully tested and you are willing to sign it off

Well, after 7 month nothing was backported and so I went ahead. Now relbranch64 and devbranch6 are reasonably in sync (first time ever) and Matteo is intensively testing things in the view of GRASS 6.4.4RC1.

From lessions learned we improve with Rashad's and Luca's support also r.li.setup in G7 which is completely different (programming language and wizard style).

Please let's get this done, at least in GRASS 6.

thanks, Markus

comment:19 by hamish, 10 years ago

If the tcl/tk version is 95% there in GRASS 6 and the wx replacement only 50% there in GRASS 7 I don't mind to spend some time to make the GRASS 6 version complete before moving on.

Markus wrote:

Do you have an estimated time for this?

With the renewed user interest in these modules, I'm a working a little bit on it each day now to complete & commit my earlier cleanups & rewrite. Expect good things through the coming week.

With the exception of the small r.li.shape patch below, it's only r.li.setup that I'm working on now, so that's the only module I ask for communication and coordination on.

fyi for sync'ing between branches I'm checking with:

$ kdiff3 relbr_6_4/raster/r.li/ devbr_6/raster/r.li/

(with some custom filters to ignore .svn files and $Date$ timestamps)

Hamish wrote:

please commit to the dev branch first then backport to the stable branch only once the change is fully tested

(mostly I was concerned with r58878 which was committed out of order to the release branch but not the dev branch [thanks for now fore-porting that M])

Matteo is intensively testing things in the view of GRASS 6.4.4RC1.

I am glad to hear it. To avoid the need for double-testing, or testing code which is about to be replaced, it will be much better to test the new commits for r.li.setup in devbr6, since they will be extensive and happen there first. The r.li.setup code currently in 6.4 is not quoted for spaces and has a number of other problems, so not safe especially for Macs, and I would support to backport the current work for the 6.4.4 release.

fyi r.li.setup is still in a state of flux in devbr6, I would not be surprised by some typos and half-commits for the next few days.

From lessions learned we improve with Rashad's and Luca's support also r.li.setup in G7 which is completely different (programming language and wizard style).

I am glad for it too, since I am not very familiar with that new tool yet.

Please let's get this done, at least in GRASS 6.

I'm happy to keep on working on the r.li.setup shell scripts, one thing which would help is if someone could work on the free()s in the r.li C modules. In some of my earlier patches I looked to add some, I note in Rashad's patch some have been commented out. It's obviously a trouble spot & a reason I allowed to set workers to 1 via an enviro var. & suggested Valgrind. for example:

?? 

Index: r.li/r.li.shape/main.c
===================================================================
--- r.li/r.li.shape/main.c	(revision 58891)
+++ r.li/r.li.shape/main.c	(working copy)
@@ -89,6 +89,7 @@
 		}
 	    }
 	}
+	free(mask_buf);
     }
 
     /* calculate distance */

with respect to the recent patches, a question about sample_area_vector.sh-

I don't object to it, but am curious to learn the reason why 'v.build cdump' was replaced by 'v.category print'? Does it do something different/better? Also, what is a reasonable maximum number of cats to expect? (i.e. to know if the list should be kept in a file or in memory)

with respect to v.extract, note that @mapset isn't needed for output= map names, since it is only allowed to write new maps to the current mapset.

thanks, Hamish

comment:20 by hamish, 10 years ago

btw, you might also try osm2pgsql to load the OSM data into a PostGIS Postgres DB, then live-connect to the PostGIS db with grass 7 with v.external. for example:

​https://trac.osgeo.org/osgeo/browser/livedvd/gisvm/trunk/bin/load_postgis.sh#L85

otherwise map coordinates are stored in $MAPSET/vector in grass's vector format, and only feature attributes are stored in the DB. Map features are connected to attributes via category number.

comment:21 by hamish, 10 years ago

oops, wrong ticket

comment:22 by hamish, 10 years ago

Hi,

r.li.setup's masked_area_selection.sh, sample_area_vector.sh, and square_mouse_selection.sh are now re-written in devbr6 and ready for testing.

with the exception of the free() patch in comment:19, I've now committed to devbr6 all my uncommitted r.li patches.

Markus wrote:

r.li/r.li.daemon/worker.c L238 BUG/TODO: mask variable already contains the @mapset, but it after this line switches to @PERMANENT

perhaps related to the new mmapset/mask_mapset variable?

Hamish

comment:23 by hamish, 10 years ago

a newline-preserved version of comment:4 can be found here:

Matteo: "below you find a minimum repeatable example that I've done using the North Carolina GRASS mapset." ...

​http://article.gmane.org/gmane.comp.gis.grass.devel/55681

the two sample config files attached to this ticket both have problems. The first has extra n= s= w= e= chars in them which need to be edited out. The second is mostly empty (the original topic of this ticket).

Hamish

in reply to:  22 ; comment:24 by rashadkm, 10 years ago

Replying to hamish:

Hi,

r.li.setup's masked_area_selection.sh, sample_area_vector.sh, and square_mouse_selection.sh are now re-written in devbr6 and ready for testing.

with the exception of the free() patch in comment:19, I've now committed to devbr6 all my uncommitted r.li patches.

So with devbr6 everything from your side is ok. I will put a patch for the free() calls to devbr6. In r.li.setup, as matteo said entering a name for sample area every time in "overlay vector" options seems difficult. An addition he asked was to have the r.li.setup provides some names automatically. sample_<conf_name>_1 like that. So I think this small addition could be added in tcl/tk too apart from g.gui.rlisetup. What do you think?

Markus wrote:

r.li/r.li.daemon/worker.c L238 BUG/TODO: mask variable already contains the @mapset, but it after this line switches to @PERMANENT

perhaps related to the new mmapset/mask_mapset variable?

Hamish

comment:25 by hamish, 10 years ago

I continued with (many) fixes for r.li.setup today in devbr6, it should work a lot better now. There are many options and I didn't test them all, but hopefully the basics are working now.

I notice that sometimes duplicate entries end up in the config file, not sure why.

With the North Carolina dataset I've been testing with landuse96_28m, zipcodes_wake, and firestations as the map inputs.

I didn't try any processing, only r.li.setup.

I added automatic prefixes to the mask map names in a number of places to make it easy to use g.mremove to cleanup, etc.

For the "overlay vector" I added some text on the display to show you how many area categories there are and how many you've done, and a button to stop the yes/no dialog when you are done (in case you selected a vector with hundreds of areas). The raster mask name is now preseeded as "InputMapname_CatNumber".

I'm not sure about the free()s, need to test with a single worker and with many to make sure there are no double-frees.

give it a try :)

regards, Hamish

in reply to:  24 comment:26 by neteler, 10 years ago

Replying to rashadkm: ...

In r.li.setup, as matteo said entering a name for sample area every time in "overlay vector" options seems difficult. An addition he asked was to have the r.li.setup provides some names automatically. sample_<conf_name>_1 like that. So I think this small addition could be added in tcl/tk too apart from g.gui.rlisetup. What do you think?

Yes, that would be very important. Imagine to manually cycle through a vector layer with 1000 polygons, then you have to insert 1000 filenames manually which could simply be the category plus unique number.

Matteo: please update the NC example here in trac...

comment:27 by hamish, 10 years ago

with @PERMANENT issues fixed/avoided in devbr6, and testing a moving window sample with r.li.edgedensity and WORKERS=1, I'm now getting as far as a broken pipe error: 

*** glibc detected *** r.li.edgedensity: free(): invalid next size (normal): 0x000000000265e980 ***

...

(gdb) bt
#0  0x00007ffff702a8d0 in __write_nocancel () at ../sysdeps/unix/syscall-template.S:82
#1  0x00007ffff755061d in send (pipe=5, m=0x7fffffffa970) at ipc.c:31
#2  0x00007ffff754e05b in calculateIndex (file=0x604630 "test_mov_windows", f=0x401401 <edgedensity>, parameters=0x0, raster=0x604270 "landuse96_28m", 
    output=0x604650 "rli.mov_wind") at daemon.c:208
#3  0x00000000004013fa in main (argc=4, argv=0x7fffffffdec8) at edgedensity.c:70

...

G65:history > valgrind --tool=memcheck  $CMD
==14113== Memcheck, a memory error detector
==14113== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==14113== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info
==14113== Command: r.li.edgedensity map=landuse96_28m conf=test_mov_windows output=rli.mov_wind
==14113== 
==14113== Syscall param write(buf) points to uninitialised byte(s)
==14113==    at 0x5A028D0: __write_nocancel (syscall-template.S:82)
==14113==    by 0x54B861C: send (ipc.c:31)
==14113==    by 0x54B5EEA: calculateIndex (daemon.c:167)
==14113==    by 0x4013F9: main (edgedensity.c:70)
==14113==  Address 0x7feffc8d4 is on thread 1's stack
==14113== 
==14118== Conditional jump or move depends on uninitialised value(s)
==14118==    at 0x54B9214: RLI_get_cell_raster_row (worker.c:286)
==14118==    by 0x4017D1: calculate (edgedensity.c:189)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Syscall param write(buf) points to uninitialised byte(s)
==14118==    at 0x5A028D0: __write_nocancel (syscall-template.S:82)
==14118==    by 0x54B861C: send (ipc.c:31)
==14118==    by 0x54B8EA5: worker (worker.c:206)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x7feffb364 is on thread 1's stack
==14118== 
==14113== Syscall param write(buf) points to uninitialised byte(s)
==14113==    at 0x5A028D0: __write_nocancel (syscall-template.S:82)
==14113==    by 0x54B861C: send (ipc.c:31)
==14113==    by 0x54B605A: calculateIndex (daemon.c:208)
==14113==    by 0x4013F9: main (edgedensity.c:70)
==14113==  Address 0x7feffc8d4 is on thread 1's stack
==14113== 
==14118== Invalid write of size 4
==14118==    at 0x4E5D630: G_set_c_null_value (null_val.c:147)
==14118==    by 0x401726: calculate (edgedensity.c:176)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x6278f04 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x4016DC: calculate (edgedensity.c:169)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Invalid read of size 4
==14118==    at 0x401A72: calculate (edgedensity.c:242)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x5eb8d64 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x54B8D51: worker (worker.c:167)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Invalid read of size 4
==14118==    at 0x401975: calculate (edgedensity.c:228)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x5eb8d64 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x54B8D51: worker (worker.c:167)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Invalid read of size 4
==14118==    at 0x401A93: calculate (edgedensity.c:244)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x6278f04 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x4016DC: calculate (edgedensity.c:169)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Invalid read of size 4
==14118==    at 0x401AEC: calculate (edgedensity.c:250)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x5eb9584 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x54B8D51: worker (worker.c:167)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Invalid read of size 4
==14118==    at 0x401C87: calculate (edgedensity.c:316)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x5eb8d64 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x54B8D51: worker (worker.c:167)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Invalid write of size 4
==14118==    at 0x4E5D630: G_set_c_null_value (null_val.c:147)
==14118==    by 0x401829: calculate (edgedensity.c:197)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x6279b14 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x4017DF: calculate (edgedensity.c:192)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
--14118-- VALGRIND INTERNAL ERROR: Valgrind received a signal 11 (SIGSEGV) - exiting
--14118-- si_code=1;  Faulting address: 0x86291FA8;  sp: 0x40327be10

valgrind: the 'impossible' happened:
   Killed by fatal signal
==14118==    at 0x380362F9: vgPlain_arena_malloc (m_mallocfree.c:245)

sched status:
  running_tid=1

Thread 1: status = VgTs_Runnable
==14118==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==14118==    by 0x4E3D59E: G__malloc (alloc.c:37)
==14118==    by 0x54B9641: avl_make (avl.c:43)
==14118==    by 0x401BD6: calculate (edgedensity.c:279)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)

...

r.li.daemon/ipc.c: 

int send(int pipe, msg *m)
{
    int check;

    /* write on pipe */
    check = write(pipe, m, sizeof(msg));

    if (check > 0)
	return 1;
    else
	return 0;
}

Hamish

in reply to:  27 comment:28 by rashadkm, 10 years ago

I am not able to reproduce this bug 

export WORKERS=1
r.li.edgedensity map=geology conf=cf1 output=edege_out

cf1 contents: 

SAMPLINGFRAME 0|0|1|1
SAMPLEAREA -1|-1|0.03571428571428571|0.02631578947368421
MOVINGWINDOW

svn revision : 58937 devbr6 geology map from spearfish60 moving window configuration

Replying to hamish:

with @PERMANENT issues fixed/avoided in devbr6, and testing a moving window sample with r.li.edgedensity and WORKERS=1, I'm now getting as far as a broken pipe error: 

*** glibc detected *** r.li.edgedensity: free(): invalid next size (normal): 0x000000000265e980 ***

...

(gdb) bt
#0  0x00007ffff702a8d0 in __write_nocancel () at ../sysdeps/unix/syscall-template.S:82
#1  0x00007ffff755061d in send (pipe=5, m=0x7fffffffa970) at ipc.c:31
#2  0x00007ffff754e05b in calculateIndex (file=0x604630 "test_mov_windows", f=0x401401 <edgedensity>, parameters=0x0, raster=0x604270 "landuse96_28m", 
    output=0x604650 "rli.mov_wind") at daemon.c:208
#3  0x00000000004013fa in main (argc=4, argv=0x7fffffffdec8) at edgedensity.c:70

...

G65:history > valgrind --tool=memcheck  $CMD
==14113== Memcheck, a memory error detector
==14113== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==14113== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info
==14113== Command: r.li.edgedensity map=landuse96_28m conf=test_mov_windows output=rli.mov_wind
==14113== 
==14113== Syscall param write(buf) points to uninitialised byte(s)
==14113==    at 0x5A028D0: __write_nocancel (syscall-template.S:82)
==14113==    by 0x54B861C: send (ipc.c:31)
==14113==    by 0x54B5EEA: calculateIndex (daemon.c:167)
==14113==    by 0x4013F9: main (edgedensity.c:70)
==14113==  Address 0x7feffc8d4 is on thread 1's stack
==14113== 
==14118== Conditional jump or move depends on uninitialised value(s)
==14118==    at 0x54B9214: RLI_get_cell_raster_row (worker.c:286)
==14118==    by 0x4017D1: calculate (edgedensity.c:189)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Syscall param write(buf) points to uninitialised byte(s)
==14118==    at 0x5A028D0: __write_nocancel (syscall-template.S:82)
==14118==    by 0x54B861C: send (ipc.c:31)
==14118==    by 0x54B8EA5: worker (worker.c:206)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x7feffb364 is on thread 1's stack
==14118== 
==14113== Syscall param write(buf) points to uninitialised byte(s)
==14113==    at 0x5A028D0: __write_nocancel (syscall-template.S:82)
==14113==    by 0x54B861C: send (ipc.c:31)
==14113==    by 0x54B605A: calculateIndex (daemon.c:208)
==14113==    by 0x4013F9: main (edgedensity.c:70)
==14113==  Address 0x7feffc8d4 is on thread 1's stack
==14113== 
==14118== Invalid write of size 4
==14118==    at 0x4E5D630: G_set_c_null_value (null_val.c:147)
==14118==    by 0x401726: calculate (edgedensity.c:176)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x6278f04 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x4016DC: calculate (edgedensity.c:169)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Invalid read of size 4
==14118==    at 0x401A72: calculate (edgedensity.c:242)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x5eb8d64 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x54B8D51: worker (worker.c:167)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Invalid read of size 4
==14118==    at 0x401975: calculate (edgedensity.c:228)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x5eb8d64 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x54B8D51: worker (worker.c:167)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Invalid read of size 4
==14118==    at 0x401A93: calculate (edgedensity.c:244)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x6278f04 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x4016DC: calculate (edgedensity.c:169)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Invalid read of size 4
==14118==    at 0x401AEC: calculate (edgedensity.c:250)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x5eb9584 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x54B8D51: worker (worker.c:167)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Invalid read of size 4
==14118==    at 0x401C87: calculate (edgedensity.c:316)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x5eb8d64 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x54B8D51: worker (worker.c:167)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
==14118== Invalid write of size 4
==14118==    at 0x4E5D630: G_set_c_null_value (null_val.c:147)
==14118==    by 0x401829: calculate (edgedensity.c:197)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118==  Address 0x6279b14 is 0 bytes after a block of size 2,004 alloc'd
==14118==    at 0x4C2380C: calloc (vg_replace_malloc.c:467)
==14118==    by 0x4E3D66C: G__calloc (alloc.c:77)
==14118==    by 0x4E3D843: G_allocate_cell_buf (alloc_cell.c:66)
==14118==    by 0x4017DF: calculate (edgedensity.c:192)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)
==14118== 
--14118-- VALGRIND INTERNAL ERROR: Valgrind received a signal 11 (SIGSEGV) - exiting
--14118-- si_code=1;  Faulting address: 0x86291FA8;  sp: 0x40327be10

valgrind: the 'impossible' happened:
   Killed by fatal signal
==14118==    at 0x380362F9: vgPlain_arena_malloc (m_mallocfree.c:245)

sched status:
  running_tid=1

Thread 1: status = VgTs_Runnable
==14118==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==14118==    by 0x4E3D59E: G__malloc (alloc.c:37)
==14118==    by 0x54B9641: avl_make (avl.c:43)
==14118==    by 0x401BD6: calculate (edgedensity.c:279)
==14118==    by 0x4014C3: edgedensity (edgedensity.c:95)
==14118==    by 0x54B8E40: worker (worker.c:192)
==14118==    by 0x54B5BEA: calculateIndex (daemon.c:102)
==14118==    by 0x4013F9: main (edgedensity.c:70)

...

r.li.daemon/ipc.c: 

int send(int pipe, msg *m)
{
    int check;

    /* write on pipe */
    check = write(pipe, m, sizeof(msg));

    if (check > 0)
	return 1;
    else
	return 0;
}

Hamish

comment:29 by hamish, 10 years ago

Hi,

I'd like to settle on a common naming scheme for the sample unit raster mask maps which r.li.setup produces.

any comments on:

rli_sample.<config_file>_<type>[_<cat>]

I had used the sampled raster name, but maybe the config file is less likely to have a name-space conflict. (as config files seem locked to a single sampled raster). The G7 wx version already uses the config file in the name.

?

Markus:

Yes, that would be very important. Imagine to manually cycle through a vector layer with 1000 polygons, then you have to insert 1000 filenames manually which could simply be the category plus unique number.

It would be pretty easy to add an "auto" button to the "is this area ok? [y/n]" dialog to accept the rest as ok, if that is a reasonable thing that makes sense for the analysis methodology.

So far I just added a warning if there were more than 30 areas to consider. Once the d.menu garbage from X-buffer bug is fixed I'd put an on-screen "this looks like it will take a very long time, are you sure?" chance to abort.

I notice with NC08's zipcodes_wake and landuse96_28m that many areas are outside of the raster-to-analyze's bounding box. Is it ok to add a 'v.select op=overlap' step to only consider vector areas with data in them? How to deal with areas which are half in out-of-bbox null-space? Crop them (v.overlay instead of v.select) or leave them to be handled in the same way that a null hole in the middle of the raster would be?

Hamish

in reply to:  12 comment:30 by hamish, 10 years ago

Replying to neteler:

The fixes attached to this ticket are not yet forward-ported to G7.

done in r58961 (with some slight modifications), note included in the patch is a bugifx by Rashad in daemon.c: m.f.f_ma.x and m.f.f_ma.y were reversed in older versions of the code.

it compiles ok, but I haven't checked the results.

Currently I'm sync'ing changes from devbr6 into trunk; if there are no objections I will remove the r.li.setup Tcl/shell script/Xmonitor stuff completely in trunk.

Since the WORKERS and server/client framework has been dropped in trunk, it will be interesting to test if the segfaults go away there too. If they do, the best approach for power users who can't wait would be to run r.li.setup in G6, then symlink the ~./r.li/ user dir into the right place for G7 and run the r.li.* modules from G7.

Hamish

comment:31 by neteler, 10 years ago

AFAIK fixed in the rewrite in trunk (backported to G6 in r59304 and r59305).

Please verify.

comment:32 by neteler, 10 years ago

Matteo,

is the issue "r.li.setup generates incomplete r.li conf file" solved?

in reply to:  32 comment:33 by matmar, 10 years ago

Resolution: → fixed
Status: assigned → closed

Replying to neteler:

Matteo,

is the issue "r.li.setup generates incomplete r.li conf file" solved?

As far I'm concerned it is solved, then I close the ticket...

Matteo

Note: See TracTickets for help on using tickets.