Data2Dynamics / d2d

a modeling environment tailored to parameter estimation in dynamical systems
https://github.com/Data2Dynamics/d2d
57 stars 28 forks source link

Various smaller bugs due to new structure. Please fix and do more testing before pushing into master! #82

Closed leshaker closed 7 years ago

leshaker commented 7 years ago

So far I found:

Error in stringListChooser>readParameterAnnotation (line 112) if(S.ar.config.fiterrors == 1)

Error in stringListChooser (line 13) [anno,vals{j}] = readParameterAnnotation(liste{j}); % vals contains npara, nfitted, chi2, ... and is useful when working with a breakpoint

Error in fileChooser (line 24) out = stringListChooser(file_list, default, zeigen);

Error in arLoadPars (line 49) [~, filename] = fileChooser(pfad, 1, true);


- Warning when running `ple`

Warning: pleSave is deprecated. pleGlobals is now stored as ar.ple by using arSave

In pleSave (line 19) In ple (line 463)

to be continued...

JoepVanlier commented 7 years ago

Yeah, I agree. It would be great if whoever decides to pick up these bugs also writes a test for this functionality (since they'll be investigating how X functionality works anyway and likely have a good understanding of how to test it) and adds them to doTests in /Examples/. That way, future breakage can be reduced.

Also, doTests should be run (preferably on all three platforms) before pushing anything potentially disruptive (e.g. changes to c code, arLink, arCompile, arLoadX, arSaveX) to the master branch. It's happened more than once that things were broken which were already covered by a test. It's much easier to repair these mistakes when the new code is still being written / edited than to attempt to figure out what is going on after the fact.

leshaker commented 7 years ago

this part in the function stringListChooser

    if(isfield(S,'ar.ple'))
        if(isfield(S.ar.ple,'chi2s'))
            nple = sum(~cellfun(@isempty,S.ar.ple.chi2s));
            plestr = [' #PLE=',sprintf('%3i ',nple)];
        end
    end

is never used because S.ar is loaded from workspace_pars_only.mat and contains only parameters but not the ple...