Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make tests actually check the computation results #3

Closed
peanutfun opened this issue Nov 8, 2021 · 7 comments
Closed

Make tests actually check the computation results #3

peanutfun opened this issue Nov 8, 2021 · 7 comments

Comments

@peanutfun
Copy link

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
Copy link
Owner

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

@peanutfun
Copy link
Author

@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
Copy link
Owner

@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
Copy link
Author

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
Copy link
Author

peanutfun commented Dec 8, 2021

@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
Copy link
Owner

@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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants