barronh / DSMACC

Dynamically Simple Model of Atmospheric Chemical Complexity
https://github.com/barronh/DSMACC/wiki
GNU General Public License v3.0
19 stars 7 forks source link

Fail to pass test cases in `test/` #5

Closed Andrewyg closed 1 year ago

Andrewyg commented 1 year ago

Greeting, Recently we're trying to recreate this project (as in the fork Andrewyg/DSMACC-patched), but we kept failing the test cases provided in test/.

Upon further investigation, it seems like there's some updates after those test cases (check.*.dat) are generated, which includes additional fields, etc.

Therefore we would like to confirm whether or not is the test still relevant? Since as shown in the fork, we've made a couple minor patches in order to make the code-base compiles. And thus we're looking for means to verify if we broke the code-base or not (through those modifications).


Our run result:

$ cd test/
$ make
make geos.show
make[3]: Entering directory '/home/user/DSMACC-patched/test'
geos Timeseries Check
Passed              27/113
Failed              86/113

geos Diurnal Check
Passed              13/113
Failed             100/113

make[3]: Leaving directory '/home/user/DSMACC-patched/test'
make[2]: Leaving directory '/home/user/DSMACC-patched/test'
make mz4.show
make[2]: Entering directory '/home/user/DSMACC-patched/test'
mz4 Timeseries Check
Passed              33/100
Failed              67/100

mz4 Diurnal Check
Passed              13/100
Failed              87/100

make[2]: Leaving directory '/home/user/DSMACC-patched/test'
make cri.show
make[2]: Entering directory '/home/user/DSMACC-patched/test'
cri Timeseries Check
Passed             232/442
Failed             210/442

cri Diurnal Check
Passed             133/442
Failed             309/442

make[2]: Leaving directory '/home/user/DSMACC-patched/test'
make geos.show
make[2]: Entering directory '/home/user/DSMACC-patched/test'
geos Timeseries Check
Passed              27/113
Failed              86/113

geos Diurnal Check
Passed              13/113
Failed             100/113
barronh commented 1 year ago

I'm confused... Did it fail before or after you made changes?

Andrewyg commented 1 year ago

So my suspection is that perhaps the test cases are no longer true (or up-to-date). As looking on the history of commits, there are changes both to the main program and the initial condition to cri and geos.

In fact, I've reverted all the way back to eabcc04, and apply the same patches, which then passes all tests (available upon that commit) 100%. (Actually we've passed tests in commit a283075.) Which kind of reinforce the idea that perhaps new features/changes/reaction chains which had been added after the test case is generated might have break those test cases.

But since those test cases are generated way back ago, I'm not really sure what have changed. Therefore I would like to ask, if possible, could you re-generate those test cases? And also, perhaps, see if the compilation errors (as listed below) we've encountered also happened on your side, as this may help pin-point whether the issue is caused by varying compiler version or not.

Huge Thanks


The patches I've made are:

And those changes are needed for the program to compile at all.

P.S. The first error is actually addressed in perhaps newer version of TUV (as shown here)


Here's the gist to the changes I've made on commit a283075. And also screenshot of result of running make check in test/:

Result screenshot

barronh commented 1 year ago

You're exactly right. This happened, I think, when the JDAY was updated to be put in as JDAY_GMT. The fraction of the day is now in GMT, but before I think it was interpreted as local. That was why inputs like JDAY were clarified... And, the thata.inc include was added with better documented functions.

The problem is, you are right, that the test cases were not updated. I think maybe we should set the dates such that they start at approximately the time it would have started in the originals. Do you all have an opinion on this?

barronh commented 1 year ago

I updated the tests so that they work with the latest version. It should be noted that these "tests" are simply to prove that the answer is the same as I got. Make sure that any implementation you do makes physical sense.

Andrewyg commented 1 year ago

Thanks, after the update we've successfully passed all test cases.