mithun218 / GiNaCDE

GiNaC Differential Equation Solver
MIT License
9 stars 1 forks source link

Make tests actually check the computation results #3

Closed peanutfun closed 2 years ago

peanutfun commented 2 years ago

Description

The tests in this repository contain source files that solve certain differential equations and write the results into a file. However, the results themselves are not checked. As such, the tests only check if the differential equations can be solved without throwing an error, but this does not verify the the results are correct.

As stated in the README.md, the GiNaC library might not return the exact same output of an expression due to some unpredictable internal ordering of symbols. So it is unsafe to check the output file contents. However, one could evaluate the resulting expressions within the code. This would either require that the expressions are returned by desolve (see #2) or that a file reader is added, which reads the output files of GiNaCDE and returns a GiNaC expression that can be evaluated.

Proposal

The tests should evaluate and this verify the results of desolve at least for few known values. This requires a way of retrieving the result as expression ex.

In the documentation of GiNaC I did not find a way to compare two expressions ex. However, one can numerically evaluate them with ex::evalf. If one knows the correct result of a differential equation, one could check for specific values like its roots.

Related issues

mithun218 commented 2 years ago

@peanutfun Thanks for opening this issue. I have implemented it here.

peanutfun commented 2 years ago

@mithun218 thanks for implementing the checkSolu function! However, you did not add it to the existing tests test1.cpp and test2.cpp, and vice versa, you did not add the new tests to the CMake test configuration. Please add the new tests so that they are executed when running ctest, and add the checkSolu queries to the original test source files as well.

mithun218 commented 2 years ago

@peanutfun thanks for your quick response. I am sorry to say that currently checkSolu function is unable to check some solutions returned by GiNaCDE due to some simplification problems in GiNaCDE (we hope we can get out of this problem in the future release). So if we use checkSolu function to the existing tests test1.cpp and test2.cpp, it may report wrong test results for a correct solution. As a result, we may miss some important solutions of input NLPDE or NLODE. For this reason, we have not used the checkSolu function in the test files, and GiNaCDE also reports the solutions without using the checkSolu function.

However, in the README file, we have explained the procedure for verifying the solutions. Here we have been told to check the solutions manually using checkSolu function of GiNaCDE or Maple and Mathematica software (Here manually means we have to copy the exact solutions with the conditions of solutions (which are also provided in the output .txt file) from the output .txt file ourselves.). To illustrate the procedures of checking the solutions, we have provided some output text files and the corresponding checking files. These checking files test some solutions reported in the provided text files, and explain how to test the solutions using Maple (Maple 2019), Mathematica (Mathematica 9), and GiNaCDE software.

Finally, to check all the solutions successfully, we recommend to use Maple or Mathematica software instead of using the checkSolu function.

peanutfun commented 2 years ago

I am sorry to say that currently checkSolu function is unable to check some solutions returned by GiNaCDE due to some simplification problems in GiNaCDE [...]

This is alright. However, a test that does not perform any checks is not a test. As I initially mentioned, it is not required to have a universal function that is able to check all GiNaCDE solutions. For the test cases where checkSolu fails, you can still add the expected solution as GiNaC expression manually and check if this expression and the solution found by GiNaCDE yield the same results for a set of pre-defined parameters.

Also, this is independent from adding the new test cases that include the checkSolu function to the executables and tests registered in CMake.

peanutfun commented 2 years ago

@mithun218, thank you for implementing my proposals. I did not realize you already finished working on it. I just checked out the latest master and had a look at the tests. In general, they look good now, but it seems to me like the checkSolu function is not working correctly. In the test/checkSolu_Painlev_FIMextravar.cpp file, I changed the solution string, but the checkSolu function still returns the same value, and the test passes although the solution is wrong. Can you give this another look? Here is an example:

#include <GiNaCDE.h>

int main()
{
    const string DE = "Diff(u,x,2)+u^3*b+u*Diff(u,x,1)*a";

    // Original solution (works)
    string algebraic_solutions = "{g_0==0,a_01==0,a_00==0,b==-1/2*(a+g_1)*g_1,a_02==1/2*a+1/2*g_1}";
    string solutions = "u = 2*(2*C_+(a+g_1)*x)^(-1)";

    // Wrong solutions that still work
    string solutions_2 = "u =  2*(2*C_+(a+g_1)*x)^(-2)";  // Wrong exponent
    string solutions_3 = "u =  1 + 2*(2*C_+(a+g_1)*x)^(-2)";  // Additional summand

    std::vector<string> solution_vec({solutions, solutions_2, solutions_3});
    for (auto sol : solution_vec) {
        ex ret = checkSolu(DE, sol, algebraic_solutions);
        if(ret!=_ex0)
        {
            // We never end up here!
            return -1;
        }
    }

    return 0;
}
mithun218 commented 2 years ago

@peanutfun Many many thanks for detecting this problem of the checkSolu function. I have noticed the problem and resolved the issue here. checkSolu function now works fine. Please check this.

peanutfun commented 2 years ago

Very good, checkSolu now appears to work correctly. Thank you! Together with the automated tests, this resolves the original issue.