Ensure that secondary/tertiary stratum IDs are not being saved as 0#33
Ensure that secondary/tertiary stratum IDs are not being saved as 0#33
Conversation
|
I also feel like this solution is maybe not complete as rows with primary stratum not in the initial conditions should probably not be included in the output stratum state datasheet at all. Also the min/max age is being reported as 0, where they should be NULL. These issues don't really have an effect on the final output, other than taking up disk space so I didn't fix them in this PR in the interest of time, but let me know if you think it's worth fixing. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors how secondary and tertiary stratum IDs are collected and used when writing summary state data, moving from integer dictionaries to nullable lists and introducing null-coalescing proxies to avoid zero values being saved in output keys.
- Replace
CreateSecondary/TertiaryStratumDictionarywith methods returningList<int?> - Iterate over
int?lists and introducessProxy/tsProxyvia??instead of raw zero - Update lookup key construction to use nullable proxies
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
alexembrey
left a comment
There was a problem hiding this comment.
I think that maybe all that needs to be done is something like this on line 1650:
OutputStratumState oss = new OutputStratumState(
dt.StratumIdSource,
ss == 0 ? null : ss,
ts == 0 ? null : ts,
iteration,
timestep,
dt.StateClassIdSource,
0, 0, 0, 0.0);
This would avoid the changes from Dictionary to List which will make things slower.
|
@alexembrey I made the requested changes to keep the dictionaries (although I had to do it slightly differently because it wouldn't let me directly convert ss and ts to null). |
alexembrey
left a comment
There was a problem hiding this comment.
The new code looks good to me, so if it fixes the problem in testing then I think all is well.
This PR fixes a bug that resulted in some rows of the Output Stratum State datasheet containing 0s instead of NULLs for the secondary and tertiary stratum IDs. These IDs get saved as 0 when the primary stratum in the row does not exist in the initial conditions distribution, but there are still transition pathways defined for the primary stratum. The IDs being 0 results in the following error when trying to export the datasheet from the command line (and consequently rsyncrosim and pysyncrosim):