bugPSPP - Bugs: bug #37999, RANK assert-fails with TEMPORARY

 
 

bug #37999: RANK assert-fails with TEMPORARY

Submitted by:  Ben Pfaff <blp>
Submitted on:  Sun 30 Dec 2012 04:56:17 AM UTC  
 
Category: Syntax ParserSeverity: 5 - Average
Status: FixedAssigned to: None
Open/Closed: ClosedRelease: Before 0.8.0
Effort: 0.00

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sat 02 Feb 2013 05:40:50 PM UTC, comment #9:

Fixed on master with commit 97de936d2 (RANK: Add support for temporary transformations.).

Ben Pfaff <blp>
Project Administrator
Fri 01 Feb 2013 06:04:30 AM UTC, comment #8:

I posted a series of patches that fix the problem to pspp-dev. They are ready for review.

Ben Pfaff <blp>
Project Administrator
Thu 31 Jan 2013 07:09:45 AM UTC, comment #7:

The bug37999 branch at git://benpfaff.org/pspp is getting pretty close. I think it'll probably take about another hour of cleanup (tomorrow?), then I'll post it for review.

Ben Pfaff <blp>
Project Administrator
Sat 26 Jan 2013 07:22:30 PM UTC, comment #6:

The bug37999 branch at git://benpfaff.org/pspp appears to work with TEMPORARY on RANK, but it needs cleanup and more testing (including unit tests).

Ben Pfaff <blp>
Project Administrator
Thu 24 Jan 2013 06:30:43 AM UTC, comment #5:

I now have a rewritten RANK that passes all of the existing tests and should be much easier to convert to work with TEMPORARY. It needs cleanup and some memory leak fixes before I can reasonably propose it for inclusion.

Ben Pfaff <blp>
Project Administrator
Tue 22 Jan 2013 05:04:11 AM UTC, comment #4:

I've made major progress on fixing this up. It isn't ready to post yet, but I hope for that soon.

Ben Pfaff <blp>
Project Administrator
Tue 08 Jan 2013 09:47:50 AM UTC, comment #3:

There are several problems, which I think need to be dealt with separately (but perhaps concurrently).

1. TEMPORARY needs to create an internal variable, which records
the original positions of the cases.

2. RANK currently is implemented in a rather funny way with regard
to splits. It performs its initial sort before considering
splits. This was done as an optimisation effort - if the split
variable and the rank variable happen to be the same, then only
one sort is required. However this way of doing things makes
the procedure hard to maintain. I think we should remove this
optimisation and tidy up a bit before trying to fix the problem.
After the problem is fixed, we can consider optimisation.

3. RANK currently creates an "internal" variable (called
$ORDER). This will no longer be required in the case of
TEMPORARY, because it will have been done by [1]. But if
there is no TEMPORARY in effect, we will still have to do it
in RANK itself. We need to make sure that in both cases, it
is done consistently (ideally using the same code).

John Darrington <jmd>
Project Administrator
Thu 03 Jan 2013 08:51:12 PM UTC, comment #2:

jmd and I talked over approaches a bit in IRC today. Here's the log:

jmd: blp: I think we might need some new functions to deal with temporary transformations to properly fix this RANK problem.
10:20
blp: jmd: Yes, that's why I've been looking at other things ;-)
blp: jmd: It's probably going to require nontrivial changes.
jmd: I've been quietly worring about something like this with RANK for a while now.
blp: I've been thinking of other ways to implement data anyhow.
blp: But those are probably inappropriate to fix this problem, since they require large whole-system changes.
blp: One kind-of-inefficient way to solve the problem would be to, at every use of TEMPORARY, introduce a new system variable that indexes cases.
jmd: Well I think we should try to find a stop-gap way of fixing the symptom, even if it's not particularly elegant.
blp: It's kind of inefficient since in most cases the variable won't be needed, but it should allow us to solve the problem.
blp: What do you think of that as a stopgap?
10:25
jmd: we could try it.
jmd: Isn't there already a $CASENUM system variable?
blp: $CASENUM isn't a real variable, the expressions code just synthesizes it.
blp: It doesn't count cases in a consistent way, either, in the sense that a SELECT IF before an expression will not cause $CASENUM to skip values.
blp: We'd want consistent case numbering at the point where we hit TEMPORARY, I think.

Ben Pfaff <blp>
Project Administrator
Sun 30 Dec 2012 06:11:17 PM UTC, comment #1:

Zoltan provided the following example, with the attached SPSS output:

(file #27167)

Ben Pfaff <blp>
Project Administrator
Sun 30 Dec 2012 04:56:17 AM UTC, original submission:

The following assert-fails:

Reported by Zoltan Fabian.

Ben Pfaff <blp>
Project Administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #27167:  spss_output.txt added by blp (24kB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jmd (Posted a comment)
  • -unavailable- added by blp (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 02 Feb 2013 05:40:50 PM UTCblpStatusReady for Test/Review=>Fixed
      Open/ClosedOpen=>Closed
    Fri 01 Feb 2013 06:04:30 AM UTCblpStatusConfirmed=>Ready for Test/Review
    Sun 30 Dec 2012 06:11:17 PM UTCblpAttached File-=>Added spss_output.txt, #27167
    Sun 30 Dec 2012 04:56:30 AM UTCblpReleaseNone=>Before 0.8.0

    Back to the top


    Powered by Savane 3.1-cleanup