kdahlquist / GRNmap

Gene Regulatory Network modeling and parameter estimation
BSD 3-Clause "New" or "Revised" License
4 stars 3 forks source link

Create tests for existing LSE function #313

Closed kdahlquist closed 7 years ago

kdahlquist commented 7 years ago

Before we tackle #310, we need test coverage for the LSE function.

dondi commented 7 years ago

Based on @cazinge's initial code inspection, it looks like an LSE test suite does not yet exist, so it will need to be put together.

im-deepfriedwater commented 7 years ago

These variables change within general_least_squares_error.m and are likely candidates for testing.

im-deepfriedwater commented 7 years ago

These variables change within lse.m and thus are viable candidates for testing so far.

trixr4kdz commented 7 years ago

Writing the tests for the LSE function is proving to be a bit tricky since we are not 100% sure of the correctness of the outputs. This circles back to what we discussed on Wednesday's meeting of the stochasticity/non-determinism of the Matlab functions that we are using. We can, however, assert that the current output of the function does not change during our implementation of the nested data structure. As a result, our idea of trying to make tests for LSE goes back to how I originally tried to write tests 2 summers ago. which is to run GRNmap using the 16 different parameter combinations and using those to check if our changes to the code dramatically affected our outputs.

@cazinge proposed that, since this is dependent on issue #323 which creates a roadblock for us, we should work in parallel to make the transition into the new data structure, i.e., create a temporary function that uses the nested structure. Admittedly, this does not conform to TDD but because we won't be changing LSE directly and will instead be putting making the transition in some temporary file, we can take advantage of our meeting time to begin implementation. We can address this issue more in-depth in the next meeting.

trixr4kdz commented 7 years ago

For future reference, here is a (somewhat) helpful link to the LSE formula:

im-deepfriedwater commented 7 years ago

Dr. Fitzpatrick gave us a run through of what is happening in general_least_squares_error. Easy General Least Squares Error Calculations

kdahlquist commented 7 years ago

@cazinge and @jtorre39 got a top-down briefing (nano-symposium) on the LSE function from @bengfitzpatrick and the group confirmed that it is within reach to create a data set against which to test the correctness of the function. @trixr4kdz should get the same background sometime, and the group can potentially meet with @bengfitzpatrick again on Friday.

Based on this knowledge, the task of writing isolated LSE unit tests can proceed.

kdahlquist commented 7 years ago

I will also note that we are still going to do TDD and write the tests first and not try to implement the new data structure in parallel as suggested by @cazinge. Test files with "artificial-data" in the filename have test data that can be used (in the text fixture, not by running the actual input file).

dondi commented 7 years ago

@dondi and @bengfitzpatrick took a look at the work in progress for the LSE unit tests and gave them a thumbs up. One to-do item is to round the test values in theta; the rest can stay the same.

The general approach for unit tests is to decouple them as much as possible from ancillary functionality such as I/O and parsing. Thus, filling out the data structure manually, even if data are just copied from the spreadsheet, is the proper setup for these tests.

dondi commented 7 years ago

To close this issue, we need to achieve sufficient code coverage. @cazinge and @trixr4kdz reported that a code coverage tool came in at 60%; they need to verify that the measurement is isolated to just coverage of gLSE correctness. If so, we can use this as one measure of sufficient coverage.

The second possibility is to list the known remaining cases. Ideally the code coverage tool and our sense of which cases are covered are congruent:

im-deepfriedwater commented 7 years ago

The biggest relative error that we're getting is 9x10^-8 for incrementing a cell data for items in the above checklist 2-4 so we decided to compare the relative error with 1x10^-7 because at this point it is only due to precision errors as we are not sure how much to round.

trixr4kdz commented 7 years ago

I ran our code coverage for just gLSE and it turns out that the coverage is around 75%. codecoverageforglse

trixr4kdz commented 7 years ago

If we run all of our test suites, the overall code coverage is below: allcodecoverage_part1 allcodecoverage_part2

im-deepfriedwater commented 7 years ago

Added four test cases in a pull request which @trixr4kdz will look at. They are increment a single data cell by 2, increment two data cells by 1 each, trying with alpha == 1 and alpha ==2 with manual penalty_out calculations.

trixr4kdz commented 7 years ago

I am unable to assert the strain_x1 part of the output of gLSE since this output is not the same as the input like we thought from our previous meeting.

For instance, when I tried to test the 3-gene, 3-edge case, strain_x1 came up as a 3x3 matrix of all 1s. The strain_x1 for the 4-gene, 6-edge control case came up as having a 4x4 matrix of varying values.

trixr4kdz commented 7 years ago

I've added tests where there are 2 strains as input instead of only 1 strain. Something that I've noticed when I was checking for relative error of L is that the relative error changed slightly for some tests when we I added the second strain. I put the actual relative error as comments in the GeneralLSETest. More specifically,

More specifically, the errors changed when I tested adding 1 to 2 different cells in 2 different strains ( testLIsCorrectFor4Gene6EdgesTwoStrainsAdd1ToSameStrains and testLIsCorrectFor4Gene6EdgesTwoStrainsAdd1ToDifferentStrains of GeneralLSETest).

Changes are in pull request #339.

kdahlquist commented 7 years ago

Assuming this is closable.