Create model_inputs_demo.xlsx

[BHagedorn-IDM]

This is the file that we will use for training purposes and I will use for my working videos.

[celiot-IDM]

@BHagedorn-IDM : first question. The InitializePopulation() function defaults to reading the sheet named TotalPop. The model_inputs_demo.xlsx spreadsheet doesn't have one. That's not necessarily a problem ... this code works just fine:

library(pacehrh)
pacehrh::SetInputExcelFile("model_inputs_demo.xlsx")
pacehrh::InitializePopulation(popSheet = "CountryPop")

But is this what you intended?

[celiot-IDM]

@BHagedorn-IDM : next problem.

The scenario fields DeliveryModel and Geography_dontedit are not present.

As things stand InitializeScenarios() fails because those columns aren't there.

I propose the following:

• Remove the checks for the Geography_dontedit field. It's never used in the code, and probably should never have been in the set of expect fields.
• Set the DeliveryModel field to blanks if it isn't specified in the spreadsheet, and put a check at the start of the cadre post-processing to detect non-blank DeliveryModel values and bail if they aren't there.

[celiot-IDM]

@BHagedorn-IDM : We have an oops. The sheet_PopValues column actually refers to the population rates sheet, not the population demographics sheet. This configuration fails because the rates sheet is actually called PopValues.

But going back to an earlier comment on this PR ... I wonder whether this happened because you were expecting (reasonably!) that the Scenario sheet would have a column for the starting population demographics. As mentioned above, the starting population demographics can be set in script code (pacehrh::InitializePopulation(popSheet = "CountryPop")), but perhaps this bug is telling us we're missing a column from the Scenarios sheet.

[celiot-IDM]

@BHagedorn-IDM : another problem/challenge. The SeasonalityCurves table should be the other way round.

In other words, this ...

... should really be this:

There are several reasons:

• It's standard practice for columns to represent fields and rows to represent data tuples.
• It's hard to validate whether the table is correctly laid out if there's no predefined column set. In most of the other tables we know which columns to expect, and we can then assume any other columns were put there for the user's own purposes. But as is there's no way to determine which columns contain real data without checking whether the column heading is cross-referenced from the Seasonality Offsets table. We do referential integrity checks like this at various places, but doing one when the table is first loaded would create load order dependencies. Better for the initial load to limit itself to a basic sanity check of the table format, with more complex referential checks happening later.
• It's bad practice for column names to contain data, in this case the index name for the curve. Note how in the alternative format there's an explicit ID field. (Under the covers, one of the first things the code does is to transpose the table so the curve names become available as values on which to do a join to the Tasks table. Changing the sheet format would actually simplify the code.)

Obviously this would be a breaking change, so I'm not going to do anything until we make a considered decision.

[celiot-IDM]

@BHagedorn-IDM : Another one ... has the time come to completely remove FTERatio as a computation type? The FTERatio column isn't in the model_inputs_demo spreadsheet. I can either remove the code that depends on it, or keep the code but make the FTERatio column optional. (Or you can add it to the demo Task Values sheet, but I'm guessing that's not what you really want.)

[celiot-IDM]

PR #243 addresses the challenges listed above.

• The starting population sheet needs to be set by InitializePopulation(popSheet = "name") if the sheet isn't the default ("PopSheet").
• The Scenario field Geography_dontedit is ignored if present.
• The Scenario field DeliveryModel is now optional, and is assigned default values if not present in the Scenarios sheet.
• The Scenario field sheet_PopValues needs to be corrected in the spreadsheet to point to population rates sheets, not population numbers sheets.
• The Tasks field FTEratio is now optional, and is given default values if not present.
• I would like to discuss changing the format of the SeasonalityCurves sheet at some later time.

[celiot-IDM]

See previous comment:

The Scenario field sheet_PopValues needs to be corrected in the spreadsheet to point to population rates sheets, not population numbers sheets.

[rhanIDM]

@celiot-IDM : Thank you for suggesting change the SeasonalityCurves table to the other way around. That makes a lot of sense. I created new issue #245 for this breaking change.

[celiot-IDM]

Fixes to Scenarios/sheet_PopValues confirmed.
Changes in PR #243 support this spreadsheet.
Future work still to be done to: flip SeasonalityCurves sheet (#245 ), and confirm formatting for the sheet currently called "Overhead_filter" (#244). (Rui and I also discussed improvements to the Cadres sheet related to #51)
But for now this version can be merged.

[celiot-IDM]

New version is OK.