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 Parser Severity:  5 - Average
Status:  Fixed Assigned to:  None
Open/Closed:  Closed Release:  Before 0.8.0
Effort:  0.00

Add a New Comment(Rich Markup)
   

You are not logged in

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

 

( 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 Files:
   
   
Comment:
   

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

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

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

    Only logged-in users can vote.

     

     

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-02-02 blp StatusReady for Test/Review => Fixed
        Open/ClosedOpen => Closed
    2013-02-01 blp StatusConfirmed => Ready for Test/Review
    2012-12-30 blp Attached File- => Added spss_output.txt, #27167
    2012-12-30 blp ReleaseNone => Before 0.8.0

    Back to the top


    Powered by Savane 3.3