correct number of qlat dataframe columns

[kumdonoaa]

In mc_reach.pyx, the required number of qlat dataframe column, , should be equal to or larger than total simulation time divided by qlat column interval in time. But, in that line merely counts simulation time steps. So, instead of nsteps, math.ceil(dt*nsteps/dt_qlat), where dt=simulation time interval [sec] and dt_qlat=lateral flow record time interval [sec], need to be used.

Also, including may make passing correct qlat column values complicated. For example, with 1hr lateral flow data and dt=300 sec, qts_subdivision should be 12 while 6 when dt=600 sec because qts_subdivisions = dt_qlat/dt in this case. Instead of changing qts_subdivision according to different values of dt or dt_qlats, replacing by int ((timestep-1)*dt/dt_qlat) makes the indexing work exactly the same. Even better, we don't have to change the value of qts_subdivisions when dt or dt_qlat changes.

Current behavior

Expected behavior

Steps to replicate behavior (include URLs)

Screenshots

[hellkite500]

I pushed a fix for this in #592. The logic is in fe6f8ff, does that look more approriate?

[hellkite500]

@groutr @jameshalgren there has been some discussion recently around qts_subdivisions and what its intended use is and whether it is strictly required or if it can/should be essentially inferred from the input series. Would either of you have any comments about this off the top of your head? It is relevant here, but we can also move that to its own issue if we want, but this made me think of the disscussion.

[jameshalgren]

Good question. We had the parameter there for early testing with un-registered input time series with no time metadata from which to infer the relationship to the simulation timesteps.

A related question that we wrestled with early on was whether to feed the array of qlateral values to the compute kernel already expanded out for all of the intermediate time steps or to keep it in the original hourly form and do the expansion in the kernel. If we feed in the hourly values, we need either a qts_subdivisions or a qts_time_step to connect the qlaterals to the simulation timestep. (In other words, even if we infer the value, we would still need to pass it to the kernel.)

Maybe a separate issue on the deeper question of kernel input marshalling would make sense.