search

smiths / caseStudies

Case studies of (manual) documentation for scientific computing software
3 stars 2 forks source link

SolarHeatingWaterOnly/CCode: PCM Occurance in Test Comparison Code #63

Open elwazana opened 6 years ago

elwazana commented 6 years ago

https://github.com/smiths/caseStudies/blob/a7e8172200744cf297aab9ab861c72a2befda9d7/CaseStudies/SolarHeatingWaterOnly/CCode/Testing/src/compareMatlabTest.c#L15-L28

Lines 20-23 use a something called PCM_ErrorM, I'm not sure if this is a misnaming error or a PCM function made it into the noPCM code. This occurs in every TEST(~~~) within the code as well, so I have linked to the whole file below.

https://github.com/smiths/caseStudies/blob/master/CaseStudies/SolarHeatingWaterOnly/CCode/Testing/src/compareMatlabTest.c

elwazana commented 6 years ago

I've also found more within SolarHeatingWaterOnly/CCode/*

smiths commented 6 years ago

Your instinct is correct @elwazana. PCM should not be mentioned in the Solar Water Only Test cases. This looks to me like the test cases for noPCM were not completed correctly. The Solar Water Only would have started with the PCM solution. Apparently not enough care was taken in changing the testing code. My guess is that this code doesn't even work.

I just had a look at the C folder and running test make fails. (I haven't investigated why.) The tests that did run showed PCM in the output text.

For issue #75 the test cases are being updated for noPCM. When the update is done, all references to PCM should be removed. The test cases comparing to Matlab (and FORTRAN) should be removed. We don't need to do this with noPCM, since, as we've discussed before, we actually can calculated, in closed-form, what the true answer is supposed to be.

We want noPCM to be as simple as possible. It should be simpler than PCM, especially for testing.