Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run a single model for a single timestep #404

Merged
merged 37 commits into from
Feb 28, 2020

Conversation

tomalrussell
Copy link
Member

@tomalrussell tomalrussell commented Feb 24, 2020

This will close #402 - work in progress.

The main thing here is the introduction of new smif CLI subcommands.

Run smif run <modelrun> --dry-run to see how to use these subcommands to
step through running a model run.

Essentially there are three steps:

  • smif decide sets up decisions and reports on the timesteps and decisions
    that the DecisionManager says should run
  • smif before_step initialises a model - this should only need to be run once
    per modelrun, and not all models do work in this step
  • smif step runs a single model for a single timestep and single decision,
    setting up the data handle and calling Model.simulate

TODO:

  • checking/thinking for robustness - may write up properly handling decisions as a separate issue
  • integration testing
  • fix unit tests
tests\cli\test_cli.py ....F.................
tests\controller\test_modelrun.py ..FF........
tests\controller\test_scheduler.py ......F..FF
tests\data_layer\test_data_handle.py .........................F.........
tests\model\test_sector_model.py F.........

Prefer extracting single-step run method, and calling it from scheduler
at each step. Seems a simpler implementation, rather than threading
this run-single-step behaviour top-down through the scheduler.

This reverts commit 4d92ac5.
This defers model (class/wrapper implementations of SectorModel) loading to
a single place, just before calling model.simulate or model.before_model_run,
and gives a relatively simple method to call to run a model for a single step.

Towards having a 'smif run single-model-single-step' CLI command, and
also in support of model/scheduler environment separation.
Fixes circular import and emphasises the two routes into
running models:
- execute_run reads a full model run configuration and then
  schedules all steps to run to completion
- execute_step runs a single model for a single step and
  assumes that all preceding steps have run (e.g. previous
  timestep has run, other models have produced necessary
  outputs)
To mirror simulate and before_model_run, and to avoid too many
arguments/options for each.
Print a summary of individual (decide/before_step/step) commands in a runnable
order.
These are the only runnable items - scenarios and scenario dependencies are
useful in knowing data dependencies but are irrelevant for run order.
Enable further work in making DecisionManager resumable - that is, run
'smif decide modelrun --decision 0' then run the models, then run
'smif decide modelrun --decision 1' to set up some further decisions for
further simulation, and so on.
Run 'smif run <modelrun> --dry-run' to see how to use these subcommands to
step through running a model run.

Essentially there are three steps:
- 'smif decide' sets up decisions and reports on the timesteps and decisions
  that the DecisionManager says should run
- 'smif before_step' initialises a model - this should only need to be run once
  per modelrun, and not all models do work in this step
- 'smif step' runs a single model for a single timestep and single decision,
  setting up the data handle and calling Model.simulate
This is unused - may be worth stripping out if there are no active plans
to develop database backend.
With round-trip testing and in particular single-level MultiIndex handling.
@codecov
Copy link

codecov bot commented Feb 25, 2020

Codecov Report

Merging #404 into master will decrease coverage by 0.05%.
The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #404      +/-   ##
==========================================
- Coverage   70.91%   70.85%   -0.06%     
==========================================
  Files          64       65       +1     
  Lines        5717     5758      +41     
  Branches      730      736       +6     
==========================================
+ Hits         4054     4080      +26     
- Misses       1559     1572      +13     
- Partials      104      106       +2
Flag Coverage Δ
#javascript 70.85% <87.5%> (-0.06%) ⬇️
#python 77.87% <87.5%> (-0.13%) ⬇️
Impacted Files Coverage Δ
src/smif/decision/decision.py 93.22% <ø> (ø) ⬆️
src/smif/cli/log.py 95.83% <100%> (ø) ⬆️
src/smif/data_layer/data_array.py 87.24% <100%> (+0.35%) ⬆️
src/smif/model/model.py 100% <100%> (ø) ⬆️
src/smif/data_layer/file/file_data_store.py 90.66% <100%> (+1%) ⬆️
src/smif/model/sector_model.py 100% <100%> (ø) ⬆️
src/smif/controller/build.py 77.27% <50%> (-0.67%) ⬇️
src/smif/controller/modelrun.py 88.95% <66.66%> (-3.05%) ⬇️
src/smif/controller/job/serial_job_scheduler.py 86.88% <80%> (-0.81%) ⬇️
src/smif/controller/execute_run.py 89.65% <91.66%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c5e004...5c95c08. Read the comment docs.

- ~4x speed-up (from about 2m to 30s)
- uses pytest capsys fixture to capture stdout and stderr
- quirk with logging means that object loggers seem to get disabled after
  repeated calls to setup_logging. Fix for now is to call logging.info()
  directly in the one case that would otherwise fail (checking jobs run by
  serail_job_scheduler).
@tomalrussell tomalrussell changed the title [WIP] Run a single model for a single timestep Run a single model for a single timestep Feb 28, 2020
@tomalrussell tomalrussell merged commit 239366e into nismod:master Feb 28, 2020
@tomalrussell tomalrussell deleted the feature/run_once branch February 28, 2020 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

smif step <model> <timestep> runs a model for a timestep
1 participant