salvadorgarciamunoz / kipet

Development repository for Kipet.
GNU General Public License v3.0
20 stars 17 forks source link

Test examples #10

Closed schenkch closed 6 years ago

schenkch commented 6 years ago

In the attached file you find the result of testing all the examples manually.

kipetexamplestest.txt

dthierry commented 6 years ago

@schenkch Thank you!, now we have a comprehensive list of the cases that don't work

Case file Reason
case51d (Casadi) dae_sim.py Jacobian calculation failed
case51d (Casadi) ode_sim.py dimension mismatch
case51d ode_sim2.py differential expressions not specified. Must be specified by user after creating the model
case51d ode_sim.py file init2.pkl missing
case51d sdae_sim.py file Slk.csv missing
case51d casadi_ode_sim.py fixing state variables is not allowed
case52a estimation.py data file missing filename
case52a scaled_estimation.py data file missing trim_Dij_case52a.txt
case52b estimation.py data file missing D_frame
michaels estimation.py not enough memory even on server
sawal estimation.py some exception
-- means that it doesn't work with error attached
#######################################################3
examples/case51a:
-casadi: ode_sim.py ++
         sdae_sim.py ++
-pyomo: estimation.py  ++ which file for plotting results?
        ode_sim.py ++
        scaled_estimation.py ++ which file for plotting results?
        sdae_sim.py ++
        sipopt/conf_interval_estimation.py ++ spelling mistake output
########################################################
examples/case51b:
-casadi: ode_sim.py ++
-pyomo: estimation.py ++
        ode_sim.py ++
        scaled_estimation.py ++
        sipopt/conf_interval_estimation.py ++ spelling mistake output
###########################################################
examples/case51c:
-casadi:ode_sim.py ++
-pyomo: estimation.py ++
        ode_sim.py ++
        scaled_estimation.py ++
########################################################
examples/case51d: 
-casadi: dae_sim.py -- Jacobian calculation failed 
         ode_sim.py -- dimension mismatch 
-pyomo: casadi_ode_sim.py -- fixing state variables is not allowed
        dae_sim.py ++ but Warning number of algebraic equations not equal to number of algebraic variables
        ode_sim2.py -- differential expressions not specified. Must be specified by user after creating the model
        ode_sim.py -- file init2.pkl missing
        sdae_sim.py -- file Slk.csv missing
######################################################
examples/case52a:
-casadi: ode_sim.py ++
-pyomo: estimation.py -- data file missing filename
        ode_sim.py ++ 
        scaled_estimation.py -- data file missing trim_Dij_case52a.txt
####################################################
examples/case52b:
-casadi: ode_sim.py ++
-pyomo: estimation.py -- data file missing D_frame
        ode_sim.py ++
#####################################################
examples/complementary_states:
-casadi: ode_conditional_sim.py ++
         ode_sim.py ++
-pyomo: ode_conditional_sim.py ++
        ode_sim.py ++
###################################################
examples/michaels:
-casadi: dae_sim.py ++
-pyomo: dae_sim2.py ++
        dae_sim.py ++
        estimation.py -- not enough memory even on server
    replicate_michaels ++
#################################################
examples/plain_pyomo:
    case51a.py ++
    case51b.py ++
    case51c.py ++
    ode_sim.py ++
    sawall_example.py ++
#################################################
examples/sawall:
-casadi: dae_sim.py ++
     ode_sim.py ++
     sdae_sim.py ++
-pyomo: dae_sim.py ++
    estimation.py --   File "estimation.py", line 83, in <module>
    solver_opts = solver_options)
  File "/home/cschenk/kipet/kipet/opt/ParameterEstimator.py", line 384, in run_opt
    **kwds)
  File "/home/cschenk/kipet/kipet/opt/ParameterEstimator.py", line 71, in _solve_extended_model
    sigma_sq[k] = max(sigma_sq.values())
ValueError: max() arg is an empty sequence

    ode_sim.py ++
    scaled_sim.py ++
    sdae_sim.py ++
mchlshort commented 6 years ago

Fixed the examples 52a estimation.py and 52b estimation.py and put into Branch Mike-ExamplesFixed . Some issues with both as they give k values quite far from the apparent optimal. Can provide fixed k and run successfully as was done in the original model, but I removed the fixed k and they run, just give very different k values. Updated txt file here: kipetexamplestest.txt

schenkch commented 6 years ago

@mchlshort: Do you have the sipopt folders for the examples that were still missing earlier? I modified the unittesting script and I would like to test whether it works for all examples including subfolders.

mchlshort commented 6 years ago

Hi @schenkch, I was actually just testing each example out with various inits and options in the solve function to see how robust the examples are. Problem is that some solve to sub-optimal solutions, so will not pass the assert. Also I was not really saving them as I was running them. I plan to, over the next day or 2, go through the examples and source code and add some more comments to help new users. While I do this I will try and systematically add the confidence intervals examples into the folders.

schenkch commented 6 years ago

@mchlshort Thanks. If you have to look at each file again anyway, maybe you can make sure that with the uploaded examples the following parts are included for not showing plots when running unittests:

with_plots = True
if len(sys.argv)==2:
    if int(sys.argv[1]):
        with_plots = False

after if name == "main":

and as for this example put the plot commands in an if statement:

if with_plots:
    results_pyomo.C.plot.line(legend=True)
    plt.xlabel("time (s)")
    plt.ylabel("Concentration (mol/L)")
    plt.title("Concentration Profile")

    results_pyomo.S.plot.line(legend=True)
    plt.xlabel("Wavelength (cm)")
    plt.ylabel("Absorbance (L/(mol cm))")
    plt.title("Absorbance  Profile")

    plt.show()
schenkch commented 6 years ago

I found one bug in the test script and updated it in the master branch, the ones not running now are the following with the following reasons:

Case file Reason run manually
case51d 7/7 files fail see above (We are gonna take that case out?!) fail manually as well
case52a buggy.py assertion error line 89 assert (abs(results_pyomo.P['k1'] - 0.00665) < tol) works manually
case52a scaled_estimation.py data file missing trim_Dij_case52a.txt fails manually as well
michaels dae_sim_david_time.py datafile trimmed.csv missing, file there but path problem works manually
michaels dae_sim.py datafile trimmed.csv missing, file there but path problem works manually
michaels estimation.py requires too much memory manually same issue
michaels replicate_michaels.py datafile trimmed.csv missing, file there but path problem works manually
schenkch commented 6 years ago

For the unittesting you should always set absolute paths to your files. Moving the trimmed.csv file to the data_sets folder and using the same absolute path description as in all the other files fixed the problem for the three files in the michaels folder.

dthierry commented 6 years ago

@schenkch should we change this to take relative paths? Or what do you think is the best design regarding where to put your data (fixed absolute, fixed relative, or something else)?

schenkch commented 6 years ago

The best way is to just use absolute paths because then it works for the unittesting and the manual testing but that is what is already used in most of the examples apart from the few examples above in the michaels folder. I will upload the fixed files soon.

mchlshort commented 6 years ago

Thanks @schenkch , when you do that, can you please keep them in a different folder to /examples? Something like /tests.

I would like the fully commented and non-repeated examples in a separate folder for the users

mchlshort commented 6 years ago

I have the folder that I would like to add on my pc and will upload it when I finish the documentation and commenting

mchlshort commented 6 years ago

Hi guys, so I have spent the last few days running the tests on Py 3.6 and noticed that the results differ quite a lot if you run the tests on Py 2 or 3. In response I have uploaded 3 different tests, 1 with a way smaller and more targeted list of tests, tut_examples_tests.py, in addition to test_examples.py which is the original, and test_examples1.py which is the original, minus the examples that were never really working or that took a really long time.

Let me know what you guys think, and any advice why some of the problems solve manually, but not in the tests.

schenkch commented 6 years ago

@mchlshort Which ones did you find to fail with the tests but not manually? In the table I added above the only one which I didn't fix running the tests was buggy.py in case52a. The other ones I found failing manually and with the tests.

mchlshort commented 6 years ago

Hi @schenkch I was actually referring to the new 'tut_test_examples.py' that I uploaded... Also, I noticed a lot of the tests failing when I ran it in Python 3.6, and not as many in Python 2.7 . Did you try with both?

schenkch commented 6 years ago

Thanks @mchlshort. Ok, I will check that out. No, I just ran them with Python 2.7.

schenkch commented 6 years ago

I tested all the tutorial examples manually and with the test script in python 2 and 3. For both versions I get the same results. Ex4 and 5 are the only ones failing just with the testing but working manually which is due to declaring relative instead of absolute paths again. Some other examples are failing for both but due to minor issues.

mchlshort commented 6 years ago

Ok, thanks a lot @schenkch. I fixed the Ex4 and 5 by adding absolute paths. Are we good to close the testing issues now for release of ver 1?

schenkch commented 6 years ago

Not yet. We should fix the other minor issues for the tutorial examples that do not work and I'm going to run the extended tests again to be sure but then I think we are ready to go. I just fixed Ad_6. There was the with_plots missing. These are the ones that still have to be fixed: Ad2_scaledest: A_set not defined Ad3_sdae_sim_nonabs: exp not defined, issue with exp in gaussian single peak in data_tools.py

mchlshort commented 6 years ago

oh, those are working for me. I can fix them now. I have them fixed on my version. Will update now

mchlshort commented 6 years ago

Ok, those should be working now. Think I forgot to push them at some stage. All seem good to me now... Let me know how it goes.

schenkch commented 6 years ago

Hi @mchlshort, There are still some issues with the Paper/replicate_example51a and c but I think they worked earlier. 51c requires too much memory and 51a gives an assertion error for k1 which can be fixed by setting tol to a large value but that is questionable. For 51c I tried to use another linear solver but that failed as well. What do you think?

mchlshort commented 6 years ago

Hi @schenkch, 51a is fixed by removing the comments on the solver options. I will work on 51c and then let you know what I find

mchlshort commented 6 years ago

Hi @schenkch , I think I fixed 51c, but I am getting an old error that I think has been fixed in the latest master. I just changed the initial guess for the parameters to be what the paper has and it seems to run through the variance estimator (where I assume it was running out of memory before?) Can you please check them again?

schenkch commented 6 years ago

I have tried many ways to fix 51c and it still does not work. Unless you guys have another idea, I would say that we take this example out. What do you think?

mchlshort commented 6 years ago

Hi @schenkch , I think that 'Ad_6_sawall.py' in the examples folder is the exact same problem and it solves, so we can maybe just swap it into that folder and re-name it?

The difference is just that this one uses the variance estimator as well. But the confidence intervals are rather large...

schenkch commented 6 years ago

Thanks @mchlshort. The differences are mainly the options for the parameter estimator. I set different options but the initial values from the paper and uploaded the new file. I will test everything with the whole back_to_fixes folder again but I think after that we are ready to go.

mchlshort commented 6 years ago

ok great. Once you have done that, maybe it would be a good idea to clean up the test folder so that we only have the working test file there and then we can merge with the main branch and let Larry know we are ready to go.

schenkch commented 6 years ago

Sounds good. What about the noise_studies folder in the Paper folder, can we take that out?

mchlshort commented 6 years ago

I am pretty indifferent about that. It is included, in some form or other, in the examples folder so I don't think it is crucial. But, at the same time, it was an example in the original paper so it might be worthwhile just keeping it around in case. You can decide.

schenkch commented 6 years ago

Really? Which one is it? But then I think we still have to work on it because the results do not look right to me.

mchlshort commented 6 years ago

Ok, I think I see the problem. It actually only needs fewer finite elements because the data is generated for only 4 discrete locations. If you do that, I think the results look ok. I tried 4 and 20 and both looked decent. Just remember to update both the Variance estimator and the Parameter estimator.

mchlshort commented 6 years ago

And yeah, in the example set I used this as a basis for another example, Ad3, but it is not the same application at all. My mistake

schenkch commented 6 years ago

Ok, thanks. Running the rest of the tests but looks good so far. I created a new branch from the back_to_fixes branch called without_notneededfiles where I am tidying up.

mchlshort commented 6 years ago

We can delete the 'chris_unittest' and the 'without_notneededfiles" branches and close this issue, right?

schenkch commented 6 years ago

I would keep the without_notneededfiles branch, such that we still have everything whenever we start deleting files but we can get rid of the chris_unittest one and the back_to_fixes one. I will close it whenever the last test is finished.

mchlshort commented 6 years ago

ok, cool. I think the 'back_to_fixes' is important still since it contains the most files. and we can always find something if it gets deleted from the more streamlined versions. I guess we can just keep hold of them both.