Opened 20 months ago

Closed 3 months ago

#2529 closed defect (done)

AR1 is not evaluated correctly on the test partition

Reported by: gkronber Owned by: gkronber
Priority: medium Milestone: HeuristicLab 3.3.15
Component: Algorithms.DataAnalysis Version: 3.3.13
Keywords: Cc:

Description

The AR1 model should regress on the previous model outputs in the test set (therefore the name auto-regressive).

Currently the AR1 model uses the actual value of the target variable at t-1 even on the test partition.

This is also true for the symbol "auto-regressive variable" for symbolic time-series modeling.

Attachments (1)

AR1-co2.jpg (39.5 KB) - added by gkronber 20 months ago.

Download all attachments as: .zip

Change History (17)

Changed 20 months ago by gkronber

comment:1 Changed 20 months ago by gkronber

Problematic code in AutoregressiveModeling.cs:

      for (int i = 0; i < timeOffset; i++) {
        LaggedVariableTreeNode node = (LaggedVariableTreeNode)new LaggedVariable().CreateTreeNode();
        node.VariableName = targetVariable;
        node.Weight = coefficients[i];
        node.Lag = (i + 1) * -1;
        addition.AddSubtree(node);
      }

The symbol AutoregressiveTargetVariable is treated in the same way as any other lagged variable in the interpreter

        case OpCodes.LagVariable: {
            var laggedVariableTreeNode = (LaggedVariableTreeNode)currentInstr.dynamicNode;
            int actualRow = row + laggedVariableTreeNode.Lag;
            if (actualRow < 0 || actualRow >= dataset.Rows) { return double.NaN; }
            return ((IList<double>)currentInstr.data)[actualRow] * laggedVariableTreeNode.Weight;
          }

comment:2 Changed 20 months ago by gkronber

Attached file shows unwanted behavior.

Last edited 12 months ago by gkronber (previous) (diff)

comment:3 Changed 17 months ago by gkronber

  • Milestone changed from HeuristicLab 3.3.14 to HeuristicLab 3.3.x Backlog

comment:4 Changed 8 months ago by gkronber

  • Status changed from new to accepted

comment:5 Changed 8 months ago by gkronber

The caching of target variable values in the interpreter for time series models has a bug (the values are not initialized correctly e.g. when the problem changes).

A specific version of the line chart needs to be implemented which uses the complete test partition as prognosis horizon.

comment:6 Changed 8 months ago by gkronber

r14422: added a way to calculate prognosed values for the whole test partition and added a specific implementation of the line chart for time series models

comment:7 Changed 8 months ago by gkronber

r14423: removed caching of traget variable values because it didn't work correctly.

comment:8 follow-up: Changed 8 months ago by gkronber

TODO: check if sample still produces the same values.

comment:9 in reply to: ↑ 8 Changed 8 months ago by gkronber

Replying to gkronber:

TODO: check if sample still produces the same values.

Nope...

Stacktraces: (new) MSTest: HeuristicLab.Tests.GPTimeSeriesSampleTest.RunGpTimeSeriesSampleTest Assert.AreEqual failed. Expected a difference no greater than <1E-08> between expected value <0,0209527534151996> and actual value <0,0490851910414951>.

comment:10 Changed 8 months ago by gkronber

Merge with #2718

comment:11 Changed 7 months ago by gkronber

  • Milestone changed from HeuristicLab 3.3.x Backlog to HeuristicLab 3.3.15

comment:12 Changed 7 months ago by gkronber

r14529: updated target result values in unit test (previously the interpreter was buggy)

comment:13 Changed 7 months ago by gkronber

  • Owner changed from gkronber to mkommend
  • Status changed from accepted to reviewing

comment:14 Changed 7 months ago by mkommend

  • Owner changed from mkommend to gkronber
  • Status changed from reviewing to readytorelease

OK. I still find it slightly misleading that the training and test range behave differently (estimation vs. prognosis).

comment:15 Changed 3 months ago by gkronber

r14939: merged r14422 and r14423 from trunk to stable

r14941: merged r14529 from trunk to stable

Last edited 3 months ago by gkronber (previous) (diff)

comment:16 Changed 3 months ago by gkronber

  • Resolution set to done
  • Status changed from readytorelease to closed
Note: See TracTickets for help on using tickets.