#18 A few bugs and how I fixed them

closed
nobody
None
5
2009-12-06
2009-05-04
Joanna
No

I'm not a developer so I don't know how to create a patch. But here are the snippets of code I used to fix some bugs I found in plotting.cpp. These are bugs present in the 3 May 2009 CVS version.

Bug 1: windows limit error

GDL> contour,dist(10),charsize=2
% Compiled module: DIST.
GDL> xyouts,5,5,"Hello"

*** PLPLOT ERROR, ABORTING OPERATION ***
plvpor: Invalid limits, aborting operation

*** PLPLOT WARNING ***
plwind: Invalid window limits in x.

*** PLPLOT WARNING ***
plwind: Invalid window limits in y.

This is how I fixed it:
At line 3143 of plotting.cpp (May 3, 2009 CVS version), insert these lines:

// Get viewpoint parameters and store in WINDOW & S
PLFLT p_xmin, p_xmax, p_ymin, p_ymax;
actStream->gvpd (p_xmin, p_xmax, p_ymin, p_ymax);

DStructGDL* Struct=NULL;
Struct = SysVar::X();
static unsigned windowTag = Struct->Desc()->TagIndex( "WINDOW");
static unsigned sTag = Struct->Desc()->TagIndex( "S");
if(Struct != NULL) {
(*static_cast<DFloatGDL*>( Struct->GetTag( windowTag, 0)))[0] = p_xmin;
(*static_cast<DFloatGDL*>( Struct->GetTag( windowTag, 0)))[1] = p_xmax;

(*static_cast<DDoubleGDL*>( Struct->GetTag( sTag, 0)))[0] =
(p_xmin*xEnd - p_xmax*xStart) / (xEnd - xStart);
(*static_cast<DDoubleGDL*>( Struct->GetTag( sTag, 0)))[1] =
(p_xmax - p_xmin) / (xEnd - xStart);

}

Struct = SysVar::Y();
if(Struct != NULL) {
(*static_cast<DFloatGDL*>( Struct->GetTag( windowTag, 0)))[0] = p_ymin;
(*static_cast<DFloatGDL*>( Struct->GetTag( windowTag, 0)))[1] = p_ymax;

(*static_cast<DDoubleGDL*>( Struct->GetTag( sTag, 0)))[0] =
(p_ymin*yEnd - p_ymax*yStart) / (yEnd - yStart);
(*static_cast<DDoubleGDL*>( Struct->GetTag( sTag, 0)))[1] =
(p_ymax - p_ymin) / (yEnd - yStart);
}

Bug 2: xyouts was preserving previous calls to the thick keyword

GDL> contour,dist(10),thick=3
GDL> xyouts,4,5,"hello"

gives "hello" with a thickness of 3 when it should be the default thickness
of 0. The way I fixed it:

Insert in line 1773 of the cvs version:

// pen thickness for axis
actStream->wid( 0);

Bug 3: this isn't exactly a bug, but a suggestion to improve contour.
I had originally fixed up my contour a while ago which fixed the PLPLOT error reported by someone else, but had not gotten around to posting it. slayoo has now fixed it, but here I suggest a better fix. I believe it improves the code by making it more compact, and by removing the need to create a second clevels array for the /fill keyword. In all the below, the line numbers are the ones in CVS version of 3 May 2009.

Replace line 2928 with:
//IDL does this:
zintv = (PLFLT) ((zEnd - zStart) / (nlevel+1));
//but I think this is better:
//if (e->KeywordSet( "FILL")) {zintv = (PLFLT) ((zEnd - zStart) / (nlevel));} else {zintv = (PLFLT) ((zEnd - zStart) / (nlevel+1));}

Replace lines 2937-2941 with:

DDouble offset=0.;
if (e->KeywordSet( "FILL")) { nlevel = nlevel + 1; offset=zintv;}
clevel = new PLFLT[nlevel];
clevel_guard.Reset( clevel);
//IDL does this:
for( SizeT i=1; i<=nlevel; i++) clevel[i-1] = zintv * i + zStart;
//but I think this is better:
//for( SizeT i=1; i<=nlevel; i++) clevel[i-1] = zintv * i + zStart - offset;

and delete lines 2949-2982.

Then replace all clevel_fill and nlevel_fill to clevel and nlevel (lines 3076 and 3118)

***Discussion of the comments "IDL does this" and "but I think this is better"
One must choose either the two "IDL does this" lines or the two "but I think this is better" lines for the code to work, not a mix of both.

To illustrate IDL behaviour and why I wanted to improve it, type these commands in IDL:

contour,[[indgen(11)],[indgen(11)]],nlevels=5
contour,[[indgen(11)],[indgen(11)]],nlevels=5,/fill

Without /fill, IDL plots nlevels (5) lines between the endpoints. This seems reasonable to me.
With /fill, IDL keeps the same nlevels lines and fills in the gaps such that there are nlevels (5) shades. However the shades are not evenly spread over the range of the data because IDL wanted to keep the contour lines it would have plotted without the /fill keyword. As a result, the first two bin-widths are the same shade, while the rest of the shades occupy only
one bin-width.

How I thought to improve the situation with /fill was to make nlevels be the number of shades, as in IDL, but to even out the bin sizes of the shades. See the above same commands in my version of GDL.

Of course, it is up to the devs to do what they think is best. Devs, please let me know what you choose, and I'll follow suit.

Discussion

  • Joanna

    Joanna - 2009-05-04

    oops. I said May 3, 2009, but I really meant May 4, 2009.

     
  • Joanna

    Joanna - 2009-05-31

    Well, I figured out how to make patches (I guess it wasn't that hard). I will attach the patch for all these changes in the next thread I'll post.

     
  • Nobody/Anonymous

    I applied all patches. They are in the CVS.
    Thanks.

    If there is reasonable complaint it might be necessary to remove the last patch again though.

    Marc

     
  • Sylwester Arabas

    • status: open --> closed
     
  • Sylwester Arabas

    • status: closed --> open
     
  • Sylwester Arabas

    Hello,

    I guess the following example shows what might cause reasonable complaints:
    a=dist(7) & contour,a,/fill,nl=5 & contour,a,/over,/foll,nl=5

    If bin sizes are the same with and without the /fill keyword it's possible to overlay isolines on a filled-contour plot.

    Cheers,
    Sylwester

     
  • Sylwester Arabas

    Hello again,

    I have just run into the problem described in my previous message when overlaying a line-contour over a filled-contour on a map. I'm pretty sure it's better keep the behaviour with- and without the /FILL keyword coherent (which is how IDL behaves). I'm commenting out the last patch (adding a note on this issue to the code).

    Cheers,
    Sylwester

     
  • Sylwester Arabas

    • status: open --> closed
     

Log in to post a comment.