|
|
|
|
|
Reproduce.png (6,667 bytes) |
|
|
patch.diff (2,217 bytes) |
|
|
Good catch. But who guarantees that the user wants to apply a custom fit which has the constant term at position 0 of the FitParams array? In fact, when you flip the array (polynomial ending with the constant term) the custom fit works too: Chart2FitSeries.SetFitBasisFunc(0, @FitBaseFunc_5, 'x^5'); Chart2FitSeries.SetFitBasisFunc(1, @FitBaseFunc_4, 'x^4'); Chart2FitSeries.SetFitBasisFunc(2, @FitBaseFunc_Cube, 'x^3'); Chart2FitSeries.SetFitBasisFunc(3, @FitBaseFunc_Square, 'x^2'); Chart2FitSeries.SetFitBasisFunc(4, @FitBaseFunc_Linear, 'x'); Chart2FitSeries.SetFitBasisFunc(5, @FitBaseFunc_Const, ''); with function FitBaseFunc_4(x: ArbFloat; Param: Integer): ArbFloat; begin Result := x*x*x*x; end; function FitBaseFunc_5(x: ArbFloat; Param: Integer): ArbFloat; begin Result := x*x*x*x*x; end; So, we cannot make any assumptions on fit function at index 0. What is wrong is the type TFitFuncIndex which should start with 0, not at 1, to avoid the compiler warning. This must be a left-over from the old version where custom fits were not possible. The other, more important issue is that the embedded TryFit routine does not set the FErrCode of the series, when PrepareFitParams fails. // Prepare fit parameters if not PrepareFitParams then begin FErrCode := fitNoBaseFunctions; // <-- was: fitRes.ErrCode := ... exit; end; With that fixed the user can check in the OnFitComplete event whether the fit was successful or not - in the case of the missing function pointer for the 0th basis function it would not be successful and the ErrCode would be "fitNoBaseFunctions". The problem left for me is that the OnFitComplete handler is called too often, I will have a look later. |
|
|
|
|
|
Related: I'm attaching a Reproduce2 test application. Tested with r60543: 1) Load Reproduce2 application in IDE - you will see a chart with red series 2) In Object Inspector, select Chart1FitSeries and change its FitEquation from fePolynomial to feCustom 3) Red series is still visible 4) Save the project, close the designed form and load the form again 5) Red series is no longer still visible The problem is that, after changing FitEquation to feCustom, red series should disappear immediately - because feCustom requires making calls to SetFitBasisFunc(), which is, of course, not performed when designing in IDE. The problem is in TFitSeries.InvalidateFitResults(): it resets FFitParams[x].Value, but not FFitParams[x].Func - so old functions are reused in the new (i.e. feCustom) mode; this is not good. Reloading the form finally makes the FFitParams table filled with zeros (i.e. completely empty), so series eventually disappears. When I added: for i:=0 to High(FFitParams) do FFitParams[i].Func := nil; to TFitSeries.InvalidateFitResults(), the problem disappeared. However, I'm not sure if resetting the functions should be made in all cases, when InvalidateFitResults() is called, or only sometimes. Please also see my post below. |
|
|
I can see two solutions. 1) I don't have anything against requiring the user to call also SetFitBasisFunc(0, ...). In this case, the change should be described as incompatible - with an explanation that, in most cases, the user will want to call: SetFitBasisFunc(0, @FitBaseFunc_Const, ''). 2) Maybe - instead in TFitSeries.PrepareFitParams() - FFitParams[x].Func should be initialized earlier, i.e. in the TFitSeries.InvalidateFitResults()? In this case, FFitParams[x].Func could be initialized for all modes, including feCustom. Thanks to that: - patch will NOT be incompatible, - not making any SetFitBasisFunc() calls will still work, by using the default functions - just as fePolynomial, - even in feCustom mode, some curve will be visible in IDE when designing. |
|
|
The problem is: When the user wants to fit a superposition of, say, sin(x) and sin(2 x) is he aware that there is a hidden basis function for the constant term at index 0? Does he wonder why he has to specify ParamCount as 3 although he provides only 2 functions? When he queries the fit results does he know how to get the constant value although he never specified it? Similar with the FixedParams needed when holding parameters constants - now again he explicitly must consider a parameter that he never specified. From this viewpoint I think that it is more logical when he has to add the basis function for the constant term by himself - this is contained in r60549. If he fits his data with ParamCount = 2 and basis functions sin(x) and sin(2x) only, but notices a poor agreement due to an offset he will automatically add a constant term to his model. |
|
|
Ok, this sounds reasonably. So let's require calling SetFitBasisFunc() for all indices in range 0 .. ParamCount-1. |
|
|
By the way: In tafitlib.pas, currently there is: TFitBaseFunc = record Func: TFitFunc; FuncName: String; end; TFitParam = record Func: TFitFunc; FuncName: String; Value: ArbFloat; Fixed: Boolean; end; a) As far as I remember, FuncName is used only in feCustom mode, so maybe it's not needed in TFitParam? b) Is there a need of splitting this into two records? In TFitSeries declaration, there are two arrays: FFitParams: TFitParamArray; FCustomFuncs: TFitBaseFuncArray; Both of them are assumed to have same length. So maybe this would be more simple: TFitParam = record Func: TFitFunc; CustomFunc: TFitFunc; CustomFuncName: String; Value: ArbFloat; Fixed: Boolean; end; And in TFitSeries.PrepareFitParams: if FFitEquation <> feCustom then FFitParams[i].Func := @FitBaseFunc_Poly else begin FFitParams[i].Func := FFitParams[i].CustomFunc; if FFitParams[i].Func = nil then exit; end; |
|
|
Of course. After fixing equation text to correctly deal with the constant term now, I think this issue can be considered to be resolved. |
|
|
Everything seems to be OK, I just wanted to make some note: currently, the fit demo program uses a FitBaseFunc_Const() function, while the TFitSeries.PrepareFitParams() method uses FitBaseFunc_Poly() in all cases for initialization, even for x^0, x^1 etc. I just wanted to note this to make sure that this is intentional. BTW: I could suggest better code: else begin if FFitParams[i].CustomFunc = nil then exit; FFitParams[i].Func := FFitParams[i].CustomFunc; end; |
|
|
And in tafuncseries.pas, there is an outdated comment: In case of custom fitting, the fit base functions become equal to the functions FCustomFuncs defined separately by the method SetFitBasisFunc(). The "FCustomFuncs" field no longer exists. |
|
|
Using FitBaseFunc_Const() is much more instructive to indicate that it refers to the constant term. Moreover, FitBaseFunc_Poly absolutely must be at index 0 of the function array because the array index is the exponent of the polynomial term, i.e. x^0 = 1. Or what do you think about using FitBaseFunc_Cos at index 0(cos(x*0) = 1) ;-) Applied the others. |
|
|
> about using FitBaseFunc_Cos at index 0(cos(x*0) = 1) ;-) I laughed loudly, point for you :D |
- Anonymous
