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

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

Open
elwazana opened this issue May 7, 2018 · 2 comments
Open
Assignees

Comments

@elwazana
Copy link
Collaborator

elwazana commented May 7, 2018

TEST(CompareMatlab, testCompareMatlab1){
struct parameters params;
params = load_params("Testing/src/inputFiles/C01.in");
double errTw, errTp, errEw, errEp;
double delta = 0.00001;
errTw = PCM_ErrorM("Testing/src/compareMatlab/M01.out", "C01.out", "TWat", params);
errTp = PCM_ErrorM("Testing/src/compareMatlab/M01.out", "C01.out", "TPCM", params);
errEw = PCM_ErrorM("Testing/src/compareMatlab/M01.out", "C01.out", "EWat", params);
errEp = PCM_ErrorM("Testing/src/compareMatlab/M01.out", "C01.out", "EPCM", params);
TEST_ASSERT_FLOAT_WITHIN_MESSAGE(delta, 0, errTw, "Water temperature");
TEST_ASSERT_FLOAT_WITHIN_MESSAGE(delta, 0, errTp, "PCM temperature");
TEST_ASSERT_FLOAT_WITHIN_MESSAGE(delta, 0, errEw, "Water energy");
TEST_ASSERT_FLOAT_WITHIN_MESSAGE(delta, 0, errEp, "PCM energy");
}

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

elwazana commented May 7, 2018

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

@smiths
Copy link
Owner

smiths commented Jun 1, 2018

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.

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