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

Add unit tests for Times.c #118

Closed
dschlaep opened this issue Dec 20, 2017 · 5 comments
Closed

Add unit tests for Times.c #118

dschlaep opened this issue Dec 20, 2017 · 5 comments

Comments

@dschlaep
Copy link
Member

High priority functions for which to write unit tests:

  • interpolate_monthlyValues
@dschlaep dschlaep added this to the Code testing milestone Dec 20, 2017
@nip5 nip5 self-assigned this Jun 1, 2018
@nip5
Copy link
Contributor

nip5 commented Jun 12, 2018

@dschlaep Can I changed the return value in interpolate_monthlyValues from void in order to return the interpolated daily values? This is the only way this is testable since as it stands now there are no pointer arguments either. Also the comments above the function say that the daily values are the output.

@dschlaep
Copy link
Member Author

I really don't see the problem and thus I don't see the need to change return from a cheap void to an expensive copy of dailyValues. The output values are returned in dailyValues.

Look for instance at how this function is used in SW_SKY_init:

interpolate_monthlyValues(v->cloudcov, v->cloudcov_daily);

where inputs = v->cloudcov and outputs v->cloudcov_daily.

@nip5
Copy link
Contributor

nip5 commented Jun 13, 2018

I'm not certain that that would actually update v -> cloudcov_daily since the second argument isn't a pointer so all changes to that member variable would be just locally in the function. Is there any proof in the code that this works? I ran some tests in the SOILWAT code directly after it is called (when it should have updated) and the value of cloudcov_daily does not update after the call
interpolate_monthlyValues(v->cloudcov, v->cloudcov_daily);

@dschlaep
Copy link
Member Author

Well, the second argument is an array, thus it decays into a pointer to the first array element when passed as argument to a function; the works just fine. Read up on your c (e.g., http://c-faq.com/aryptr/aryptrparam.html).

However, what you see when you only look at the first array element is a bug in the function, i.e., that it returns 0 for the first day.

swprintf("'SW_SKY_init' before: " \
  "cc_monthly[Jan] = %f, cc_monthly[Feb] = %f, \n" \
  "\tcc_daily[Jan 1] = %f, cc_daily[Jan 15] = %f, cc_daily[Feb 1] = %f\n",
  v->cloudcov[0], v->cloudcov[1],
  v->cloudcov_daily[0], v->cloudcov_daily[14], v->cloudcov_daily[31]);

	interpolate_monthlyValues(v->cloudcov, v->cloudcov_daily);

swprintf("'SW_SKY_init' after: " \
  "cc_monthly[Jan] = %f, cc_monthly[Feb] = %f, \n" \
  "\tcc_daily[Jan 1] = %f, cc_daily[Jan 2] = %f, cc_daily[Jan 14] = %f, cc_daily[Jan 15] = %f, cc_daily[Feb 1] = %f\n",
  v->cloudcov[0], v->cloudcov[1],
  v->cloudcov_daily[0], v->cloudcov_daily[1], v->cloudcov_daily[13], v->cloudcov_daily[14], v->cloudcov_daily[31]);
'SW_SKY_init' before: cc_monthly[Jan] = 71.000000, cc_monthly[Feb] = 61.000000, 
	cc_daily[Jan 1] = 0.000000, cc_daily[Jan 15] = 0.000000, cc_daily[Feb 1] = 0.000000
'SW_SKY_init' after: cc_monthly[Jan] = 71.000000, cc_monthly[Feb] = 61.000000, 
	cc_daily[Jan 1] = 0.000000, cc_daily[Jan 2] = 66.483871, cc_daily[Jan 14] = 70.354839, cc_daily[Jan 15] = 70.677419, cc_daily[Feb 1] = 65.838710

@dschlaep
Copy link
Member Author

@nip5 @CaitlinA Would you please open an issue dedicated document this bug at index 0? This seems to be a base0--base1 problem. And ideally write a test that fails, and then fix it? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants