Integrate EDGE Growth into Forecasting Model

[mishtaiwi]

Brought in the EDGE growth functions under normits_demand/models/forecasting/edge_forecast.py Call main function in normits_demand/models/forecasting/edge_replicant.py

Add the Cube export/file copying process and functions under normits_demand\utils being: c01_ExportCubeFunctions and r01_RunExportCube

[BenTaylor-TfN]

I've made some initial changes to the code detailed below:

• Tided up imports - plenty of import weren't being used
• Formatted code with black - coding standards, easier to read
• Removed some pd.options.mode.chained_assignment = None lines
• Various bit of tidying
  • All this does is supress warnings. Deal with these warnings at the source instead.
• Moved files around
  • normits_demand/utils/c01_ExportCubeFunctions.py -> normits_demand/tools/edge_cube_extractor.py
    • Hopefully this is a tool at some point - just needs wrapping in a nice class / run-function
    • Also allows linking in with the rest of the EDGE replicant in future. That is, if the setup hasn't been done yet, just run the setup tool automatically.
  • normits_demand/utils/r01_RunExportCube.py run_tools/run_edge_cube_extractor.py
    • Links into the above point, making this a simple "setup and run" script.

I've also moved all imports here: I:\NorMITs Demand\import\edge_replicant

[BenTaylor-TfN]

I'm now finished with my review/changes in the code for now. The code is now ready for a bit more of an in-depth review.

A couple of points from my review @mishtaiwi:

• variable/function naming should be all lower-case with underscores. There should never be an upper-case letter in a variable/function name.
  • Classes are a bit different. See the guide here.
• Add type hints to functions
• Run PyLint, MyPy (once type hints added), black, and PyDocStyle for general guidance on how to follow the standards.

Things for the next reviewer to keep an eye out for:

• Look for places that functionality can be replaced with framework functionality.
  • A good example of this is the edge_cube_extractor.CheckFileExists. There's already utils in the framework to do this.
• Note places code may need a refactor to fit with framework, even if not able to make changes now
• Keep an eye out for inconsistencies with the general flow of things in demand.
• Logging needs bringing into line with rest of demand so logging messages are picked up correctly.
  • I've chucked together an example in the cube extractor run/tool scripts
• normits_demand/models/forecasting/edge_forecast.py needs to be broken apart.
  • functions which exist in framework already should be used instead.
  • New/Unique functionality should be moved over to normits_demand/models/forecasting/edge_replicant.py
  • Ideally, normits_demand/models/forecasting/edge_replicant.py should have a front end and be a collection of functions to achieve their specific goal. We can then attach other EDGE stuff to this model later if we are able to publish.

Happy to send over a copy of the coding standards if you want a look. Let's focus on getting 80% of the way there for now and hit all the main points. It doesn't have to be perfect if other work needs doing.

[BenTaylor-TfN]

Noting down some methodology questions/notes from my review. I want to make sure I understand why all the design decisions have been made. @mishtaiwi - we may want a call to go through this in detail. Let's add items to the forecasting backlog after discussion

Notes

• Before working on some of these, lets focus on making sure we've got a working and approved process. We can then merge this pull request and use the TfN version of this branch to hold the working version of the code. That then gives us space to slightly break this code in order to improve the process
• The process currently prints out base and future demand as it is running. I'm aware this is added to a log file during run, but it would be nice / cleaner to get this output as a single log file
• On the base EDGE / NoRMS (with MOIRA) runs.
  • TLC file generation should stay in pre-processing step. It's important that we separate the growth process from CUBE / NoRMS if we can
  • Instead, lets output two different TLC files. One for NoRMS, one for standard EDGE
  • Lets also make it obvious that the TLC generation and matrix extraction are 2 related, yet distinct steps that can be run separately

Questions

• Perhaps it makes sense to make an "EdgeReplicant" class / model to facilitate the following questions/changes
  • How much work would it be to allow the user to input a list of years and get the process to run for all years?
• Currently there are no outputs until the very end of the process (meaning that if it crashes before the end, we get nothing).
  • Could we introduce a "Checkpointing" process where we output the matrices once they have been grown
  • Then we have a separate "output" step which aggregates the matrices to the output format
• Why does the cube extractor output 100+ matrices? Are there truly this many distinct growth factors? Can we aggregate these before input at all?
  • Could we introduce some sort of normalised naming convention that separate from NoRMS? Making it obvious which matrices are using which growth factors e.g. "hb_external_reduced_ticket...csv"
• I:\NorMITs Demand\import\edge_replicant\apply_growth\Segments2UCs.csv links input matrices to userclasses. Does it makes sense to add the growth method and internal / external to here instead of hardcoding it?
• It's currently a quite slow process. We could multiprocess it. Is there anything (memory / accessing the same data) you think would stop us from multiprocessing this?