[joss review] MATLAB tests

[jakelangham]

As part of the ongoing reviews at https://github.com/openjournals/joss-reviews/issues/6288. It is a requirement for acceptance into JOSS for your repository to contain testing 'to objectively check the expected functionality of the software'.

I can see a folder marked tests_C++/ which I will look over in due course. Are there also tests available for the MATLAB part of the library?

[jcorbino]

@jakelangham The tests in tests_C++ are built and executed at the end of the installation process for the C++ flavor of MOLE. The MATLAB flavor of our library does not count with an equivalent folder. We can create a set of tests (similar to those for the C++ case) that the MATLAB user would be encouraged to execute before using the library. Will that fulfill the requirement?

[jakelangham]

Hi @jcorbino. Yes, my view would be that there should be MATLAB scripts that are equivalent to the C++ tests (to the extent that is possible) and this is necessary to satisfy the requirement for testing. However, on looking over the C++ tests, I do not believe these are currently sufficient for verifying the correct functionality of the library.

From what I can see, the tests verify some but not all of the claimed properties of MOLE operators as stated in https://github.com/jcorbino/mole/blob/master/CSRC%20Report%20on%20MOLE.pdf and only do so for the simplest constructions (e.g. the first three are 2nd order operators on regular minimal size grid with dx = 1).

A more comprehensive set of tests would look like a set of scripts that iterated over each approximation order and the various different types of supported grids, checking each of the various calculus identities and verifying the convergence of the approximations made (e.g. by applying them to a known function and showing that the computation converges to the corresponding analytical result as dx -> 0).

[jcorbino]

Hi @jakelangham , apologies for getting back to you late. I've had a lot going on over the past month. Now that things have calmed down, I can focus on your suggestions.

[victorapm]

@jcorbino along these lines, I would like to suggest that you add a small quick-start section to the top-level README.md file about running examples and tests with your library (MATLAB and C++). The idea is to make it a bit easier for users to start get started :)

[jcorbino]

@victorapm I've added a section to the README file to reference the examples and tests. Here's a screenshot:

[jcorbino]

@jakelangham I created a folder analogous to tests_C++ with five tests for the MATLAB version of MOLE. In the README file, I invite the users to first run such tests by executing the script tests_MATLAB/run_tests.m. As per your suggestion, the fifth test performs an accuracy test.

Thanks for the suggestions!

[jakelangham]

Hi, @jcorbino. This seems like good progress to me. The tests seem sound and pass when I run them.

However, I am not convinced they fully address what I was getting at in https://github.com/jcorbino/mole/issues/12#issuecomment-1956529354 What I am concerned about is that they only verify the correctness of the simplest subset of the operators that may be constructed using MOLE.

For example, what I can see from the source is that the higher-order operators (which become increasingly complex) are dealt with via switch case statements. Therefore, the fact that the tests pass for the k = 2 cases can tell me nothing about whether there is an accidental typo in one of the higher order cases.

The guideline that I am trying to follow from the JOSS docs is that the tests should be able to 'objectively check the expected functionality of the software'. In my view, this means the tests should at least cover the available discretisation orders, as well as some of the different grids advertised, for each of the core differential operators supported. I am envisaging that this could be achieved by augmenting the existing tests to include loops that iterate over these different choices.

[jcorbino]

Thank you, @jakelangham , for your valuable feedback! I have incorporated your suggestion of augmenting the existing tests to include loops that iterate over different orders of accuracy. You can view the changes in this commit. Thanks again for bringing this to my attention! I hope this will be sufficient for the JOSS submission.

As a side note, we have just been informed that we received a grant from the NSF to expand the library. I will ensure that documentation and testing are top priorities on our to-do list.

[jakelangham]

Hi, @jcorbino. First of all, congratulations on your grant!

I'm glad to see the iteration over the operator order. Can I ask why you are not also including the 8th order operators? What about testing these operators on different grids etc? (Maybe it is just a case of moving some of the examples over into the test directory and tidying up?)

[jcorbino]

@jakelangham Thank you!

I have added 8th-order operators to the pertinent tests (the divergence should not be requested for orders > 6 due to negative weights and the absence of a norm). Regarding the different grids: Our library, for the moment, does not provide grid generation routines. We have a couple of scripts for 2D grid generation using transfinite interpolation and elliptic grid generation. However, we do not advertise this since, for now, the user is responsible for providing the grids. In the tests, I evaluate the operators for different grid resolutions. The point of the tests is to ensure that:

1. In the C++ case, all dependencies are installed and visible to MOLE.

2. The core operators (both flavors) are correct and provide the advertised order of accuracy.

We are fully aware of the potential for further expansion and improvement in our testing procedures. We are committed to working on this and enhancing our documentation. However, I would like to know if the current level of testing meets the review criteria for JOSS.

[jakelangham]

@jcorbino If the divergence should not be used at 8th order then I would suggest adding this to your documentation and/or providing some indication of this via the way the underlying code works.

But in any case, I am running out of things to pick holes in here so that probably means it's ready to go back to the editor.