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

Variability #381

Merged
merged 28 commits into from
Jun 7, 2019
Merged

Variability #381

merged 28 commits into from
Jun 7, 2019

Conversation

tlestang
Copy link
Contributor

The variability functionality (see #353) allows users of smif to perform an ensemble of model runs, based on an ensemble of scenario variants. In this case the command smif run in invoked with a batch file (#179) that contains the list of model runs.

This pull request mainly adds the smif prepare-run and smif prepare-scenario commands (#364 ).

tlestang and others added 22 commits April 26, 2019 15:51
…smif prepare. Based on a scenario file template, create multiple scenario variants
- Bit of cleaning for cli.prepare_scenario and cli.prepare_model_run
… the

upper level Store class and not the lower file store YamlConfigStore class.
…opulation_variants.yml scenario to

test variability within sample_project #364
@codecov
Copy link

codecov bot commented May 24, 2019

Codecov Report

Merging #381 into master will increase coverage by 0.34%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #381      +/-   ##
==========================================
+ Coverage   71.09%   71.44%   +0.34%     
==========================================
  Files          60       60              
  Lines        5342     5400      +58     
  Branches      661      669       +8     
==========================================
+ Hits         3798     3858      +60     
+ Misses       1448     1446       -2     
  Partials       96       96
Flag Coverage Δ
#javascript 71.44% <100%> (+0.34%) ⬆️
#python 79.11% <100%> (+0.31%) ⬆️
Impacted Files Coverage Δ
src/smif/cli/log.py 95.83% <ø> (-0.17%) ⬇️
src/smif/data_layer/store.py 92.32% <100%> (+1.03%) ⬆️
src/smif/data_layer/results.py 93.24% <100%> (+1.71%) ⬆️
src/smif/data_layer/memory_interface.py 88.81% <100%> (+0.69%) ⬆️

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 61a855e...8ed527d. Read the comment docs.

Copy link
Member

@tomalrussell tomalrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tlestang - this will be super useful for helping with larger sets of variants, and it's a neat step forward in thinking through smif's interface for manipulating all the config.

I've suggested a few changes below - apologies, it's come out as a whole pile of one-line changes, I thought I'd experiment with the GitHub interface. You should be able to skim through them and commit in a couple of batches:

  • style/clarity improvements of Store.prepare_scenario, which also remove the need for the first-item special case in the tests
  • read and write strategies in Store.prepare_model_runs, which mean the test setup needs to write strategies too

src/smif/data_layer/store.py Outdated Show resolved Hide resolved
src/smif/data_layer/store.py Outdated Show resolved Hide resolved
src/smif/data_layer/store.py Outdated Show resolved Hide resolved
src/smif/data_layer/store.py Outdated Show resolved Hide resolved
src/smif/data_layer/store.py Outdated Show resolved Hide resolved
tests/data_layer/test_store.py Outdated Show resolved Hide resolved
tests/data_layer/test_store.py Outdated Show resolved Hide resolved
tests/data_layer/test_store.py Show resolved Hide resolved
src/smif/data_layer/store.py Show resolved Hide resolved
src/smif/data_layer/store.py Show resolved Hide resolved
also removes the need for the first-item special case in the tests
- so the test setup needs to write strategies too
@tomalrussell tomalrussell changed the base branch from master to develop June 7, 2019 09:05
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.

3 participants