Unitialized basin_id

[christophertubbs]

Forcing.h gets repeated warnings on build because one of the constructors sets basin_id(basin_id), but, since no basin_id is passed, the constructor just sets the basin_id as the value it already was (which wasn't anything).

Would anyone be opposed to me setting that as 0 to ensure that it's at least getting the default value or just taking it out of the signature?

[robertbartel]

@christophertubbs, if long-term it doesn't make sense to allow default basin ids values (i.e., we eventually will need to enforce their explicit inclusion when calling constructors), I lean toward leaving it as-is to remind us there is something there that needs fixing. I wouldn't argue too much though if that got changed regardless.

Given that, I'm fine with either adding a default argument value or an additional constructor, such that this gets initialized to 0.

[hellkite500]

A quick look shows that basin_id isn't even used anywhere by the forcing object. Essentially, the identity is connected to the input file passed in to be read, and the forcing object is attached to the realization object. There may come a point where we need to pass a larger forcing data set and have a forcing object pull out just the relevant information for a given identity, but if that information doesn't exist, that should be an error condition.

[hellkite500]

Also, see the TODO here:

ngen/src/core/catchment/HY_CatchmentRealization.cpp

Line 8 in ce23388

//TODO consider passing an ID to the forcing object???

I think we should probably pass the appropriate catchment id (and rename basin_id to catchment_id to be consistent with its use across the realizations)

[jdmattern]

Yes, this is a leftover from the initial prototype. I will submit a PR now with the change to catchment_id.

I will add the 0 as default for now and put a todo for an explicit inclusion as Bobby says.