Model output includes startDate #604
Conversation
| @@ -40,6 +40,10 @@ test_that("Basic hcore functionality works", { | |||
| expect_silent(run(hc, 2100)) | |||
| expect_silent(run(hc)) | |||
|
|
|||
| # All years should appear in the output | |||
| out <- fetchvars(hc, dates = 1745:2300) | |||
| expect_equivalent(sort(unique(out$year)), 1745:2300) | |||
There was a problem hiding this comment.
Why are there two expect_silent(run) in a row after each other? Is the second run(hc) call just to run the date out to the year 2300 for all years should appear in the output? test?
There was a problem hiding this comment.
This is older code, not something I added. The model is at 2100 at that point, and those lines test that it will run to 2100 (no change) and then continue to 2300, all without error.
| // Initialize surface flux tracking variables | ||
| annualflux_sum.set( 0.0, U_PGC ); | ||
| annualflux_sumHL.set( 0.0, U_PGC ); | ||
| annualflux_sumLL.set( 0.0, U_PGC ); |
There was a problem hiding this comment.
The annualflux_sum is also set in the run component around line 360 does it really need to be set twice? Or could we remove that code block?
There was a problem hiding this comment.
It needs to be zeroed here as well, so that the initial output visitor pass — which now occurs before the component has ever run — can correctly report model state.
| // visitors that spinup is 'true' (which it's not). This is necessary because the ocean | ||
| // component hasn't had a chance to initialize its active chemistry yet, and without that | ||
| // calc_revelle() will throw an error. Kind of hacky; but the only alternative I see is | ||
| // to create a whole new done_with_spinup() signal to the components--a lot of work. |
There was a problem hiding this comment.
Not sure I fully understand what is going on here. Is the initial output 1745? Or is it any start year?
So this is just a hack that is letting the visitor output access the initial year values even though the ocean is not at equlibrium? Would it make sense to open an issue :( outlining what done_with_spinup() would look like, where, and why it would be used?
There was a problem hiding this comment.
It's a bit hacky, yes, but allows the visitor to collect model state even though the ocean chemistry has not yet been turned on. That last bit is the issue—ocean chemistry is turned on the first time the ocean is run(). Now, however, the visitor needs to output things and, because we're no longer in spinup, it tries to collect info on chemistry (and fails). So instead, we tell the visitor, one last time, that we're still in spinup.
Changes model output so that the csvOutputStream as well as
hector::fetchvarsdata now start with 1745, or whateverstartDateis.Closes #177