mikeizbicki / subhask

Type safe interface for working in subcategories of Hask
BSD 3-Clause "New" or "Revised" License
418 stars 43 forks source link

there are some law_*, defn_*, and theorem_* that are not being tested by the test suite #19

Open cdepillabout opened 9 years ago

cdepillabout commented 9 years ago

I was reading about the automated testing in the README, and I started wondering if maybe there were any law_*, defn_*, or theorm_* functions that weren't added to the test suite for one reason or another.

Using this super hacky bash script, it is possible to list these functions:

for f in $(git grep -h '^law_' src/ | sed -e 's/\([_A-Za-z0-9]\+\).*/\1/' | sort | uniq) ; do
    output=$(git grep "$f" src/SubHask/TemplateHaskell/Test.hs) ;
    [ "$?" -eq 1 ] && echo $f ;
done

Here is a list of each type of function that is not added to the test file:

law_Banach_distance
law_Banach_size
law_Bregman_nonnegativity
law_Bregman_triangle
law_Lattice_antisymmetry
law_Lattice_associative
law_Lattice_commutative
law_Lattice_reflexivity
law_Lattice_transitivity
law_Subtype_f1
law_Subtype_f2

theorem_Lattice_idempotent

defn_Eq_noteq
defn_Lattice_greaterthan

I tried adding one of these to the test file and it seemed to work. All tests passed.

Is there some reason they are not being tested? Should I add them to the test file and send a pull request?

mikeizbicki commented 9 years ago

The Lattice and Eq ones are bugs and should be added.

The Subtype ones have a really complicated type. I remember not being sure how to even test them because their laws related to so many other classes. But in principle, they can be added if there's a good way to do so.

The Banach ones can't be added yet. The laws listed correspond to the laws for the actual math class. But these laws don't actually hold for Float and Double. Most of the high end of the math hierarchy don't actually have any laws listed because of this limitation. I've toyed around with a few different ways to test operations over these types, but nothing's actually landed in the code base yet. If you have any ideas on good ways to do this, I'd love to hear them!

cdepillabout commented 9 years ago

Pull request #20 adds the following tests to the test framework:

law_Lattice_antisymmetry
law_Lattice_associative
law_Lattice_commutative
law_Lattice_reflexivity
law_Lattice_transitivity

theorem_Lattice_idempotent

defn_Eq_noteq
defn_Lattice_greaterthan

The following are still remaining:

law_Banach_distance
law_Banach_size
law_Bregman_nonnegativity
law_Bregman_triangle
law_Subtype_f1
law_Subtype_f2

I will take a look about adding them, but I'm not confident I will find a good way.

cdepillabout commented 9 years ago

I also have two questions related to the testing framework.

  1. It looks like the laws, theorems, and definitions are not being tested by default by the test/TestSuite.hs file? I had to change ropt_list_only from RunnerOptions to False to get the tests to actually run. Is there a reason that they tests are not being run by default?
  2. When actually running the tests, it looks like some of the UArray and BArray tests fail. Should I look into this?

    In our email exchange, you said:

    There's a number of tests provided semi-automatically via template haskell (see the /test folder in the repo and there's a section in the README). These automatic tests aren't quite good enough for all the edge cases in the Array/Vector code though because the code has to handle a lot of low-level memory details.

    Is this related to the failing UArray and BArray tests?

mikeizbicki commented 9 years ago
  1. Thanks for pointing this out. This was a very serious oversight in the testing suite that accidentally got incorporated. I've pushed a fix.
  2. Some of these test failures were due to incorrect laws in the classes. I think due to problem 1 you pointed out, these didn't get noticed earlier. The basic gist of the problem was that the IxConstructible class had laws to make it work with Map-like containers only, when it needed to work with both map and array like containers. I've updated these laws, and that fixed a number of the problems.

    You're right, however, that there are still problems with the way UArray of a UVector works that needs to be fixed, and that was causing some of the other problems. That will probably take a lot more digging, however, to figure out what the exact problem is.

cdepillabout commented 9 years ago

Okay, I've confirmed on my end that the improvements and fix is working. It looks like all tests are passing now.

You're right, however, that there are still problems with the way UArray of a UVector works that needs to be fixed, and that was causing some of the other problems. That will probably take a lot more digging, however, to figure out what the exact problem is.

I'll try to take a look at this. Is there anywhere I should start specifically? Any specific functions/classes you know that need more tests or may not be working?

mikeizbicki commented 9 years ago

The hard case is UArray of UVector. All the other ones are fully working AFAIK.