CAFDI-7 - Create Multi TLD Gravity Model MVP #7

wsp-mbuckley · 2023-09-07T15:51:31Z

Describe Changes

Add class to calibrate gravity model to multiple cost distributions.

CAFDI-20 - Move gravity and jacobian functions to single_Core
CAFDI-21 - implement multi_cost_distribution Class
CAFDI-25 - implement MultiGravityModelResults
CAFDI-22 - refactor _calibrate function
CAFDI-23 - implement _gravity_function
CAFDI-24 - implement _jacobian_function
CAFDI-26 - Write multiTLD tests for run
CAFDI-27 - Write tests for calibration
CAFDI-28 - Write tests for error checking

Task Checklist

Have unittests been added (testing individual functions and their edge cases)
Have integration tests been added (if applicable, to test modules of functionality)
Updated RELEASE.md to reflect changes in PR
Have new dependencies been added?
- Have they been added to either requirements.txt or requirements_dev.txt.
- Have they been added to setup.cfg

Added TODO items and plans for the Multi area gravity model.

BenTaylor-TfN · 2023-09-20T14:36:15Z

Finished #6 now. Also updated the packaging system in #8.

Rebased this off of the new main to keep the commit messages in here a bit clearer. No significant changes that you weren't already aware of.

codecov · 2023-09-20T14:37:02Z

Codecov Report

Attention: 89 lines in your changes are missing coverage. Please review.

Comparison is base (30e4c8c) 85.09% compared to head (e9672b2) 81.38%.

Files	Patch %	Lines
src/caf/distribute/gravity_model/multi_area.py	81.63%	36 Missing ⚠️
src/caf/distribute/gravity_model/core.py	26.66%	22 Missing ⚠️
src/caf/distribute/gravity_model/single_area.py	82.75%	20 Missing ⚠️
src/caf/distribute/utils.py	52.17%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main       #7      +/-   ##
==========================================
- Coverage   85.09%   81.38%   -3.71%     
==========================================
  Files           7        9       +2     
  Lines         416      634     +218     
==========================================
+ Hits          354      516     +162     
- Misses         62      118      +56

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Added TODO items and plans for the Multi area gravity model.

…North/caf.distribute into multi_tld_mvp

Moved `_gravity_function` and `_jacobian_function` methods from base gravity model to single area and replaced with abstract method definitions.

Will need a lot of testing and tinkering

Params aren't changing after 4 or 5 outer iterations and it's quite quick: - Tidy code - Better convergence checks - Translate from df to array - Initial guess of params? - Work out zero div error

Distributions converge as well on a final furness as they do in internal loops. This is good for most but bad for external, trying to work out if this is inherent to the distribution or a problem with the code.

Currently only one function for infilling cost matrices

This all needs testing. Especially make sure everything is always in the right order

This could be used to improve performance on large matrices in MVP+

Old methods deleted, return to previous commit to find them if needed

tbc

Fixed index page to match style of toolkit. Added pages for new classes/modules

Refactored to avoid ugly inheritances from core

…ity_function

Co-authored-by: Ben <[email protected]>

…North/caf.distribute into multi_tld_mvp

BenTaylor-TfN

Dont forget to add to .gitblame to ignore the isort changes, just so you're not the one in the blame if something goes wrong with the isort changes!

There's still a few comments that need addressing from the previous review

src/caf/distribute/utils.py

tests/gravity_model/test_multi_area.py

docs/conf.py

Co-authored-by: Ben <[email protected]>

…North/caf.distribute into multi_tld_mvp

isaac-tfn · 2024-01-17T11:14:27Z

@BenTaylor-TfN I think I've made those changes now

Added module and comments for Multi area GM

69a77a8

Added TODO items and plans for the Multi area gravity model.

wsp-mbuckley added the enhancement New feature or request label Sep 7, 2023

wsp-mbuckley self-assigned this Sep 7, 2023

BenTaylor-TfN force-pushed the multi_tld_mvp branch from 69a77a8 to c1a279f Compare September 20, 2023 14:34

Added module and comments for Multi area GM

5eeac31

Added TODO items and plans for the Multi area gravity model.

BenTaylor-TfN force-pushed the multi_tld_mvp branch from c1a279f to 5eeac31 Compare October 26, 2023 16:05

wsp-mbuckley and others added 22 commits November 10, 2023 09:20

Merge branch 'multi_tld_mvp' of https://github.com/Transport-for-the-…

1e25262

…North/caf.distribute into multi_tld_mvp

Moved gravity and jacobian methods to single area

3e81f07

Moved `_gravity_function` and `_jacobian_function` methods from base gravity model to single area and replaced with abstract method definitions.

Changed furness to work for non-square matrices

61075b3

Rough structure of multi-tld

d2bae20

Will need a lot of testing and tinkering

Seems to work roughly

1b5037c

Params aren't changing after 4 or 5 outer iterations and it's quite quick: - Tidy code - Better convergence checks - Translate from df to array - Initial guess of params? - Work out zero div error

Pushing changes to work on laptop because no internet

fe87b7b

Crude convergence check

d4b6934

Distributions converge as well on a final furness as they do in internal loops. This is good for most but bad for external, trying to work out if this is inherent to the distribution or a problem with the code.

Ready to mp

9eef536

Warning raised for zeros in the seed matrix

5180db5

New utils module

1cac8d7

Currently only one function for infilling cost matrices

Written new jacobian to account for all mu and sigma. Untried.

24fbdff

New gravity written

6d1f15c

This all needs testing. Especially make sure everything is always in the right order

Triply constrained furness is commented out

ca2e3b9

This could be used to improve performance on large matrices in MVP+

Updated methods to perform jacobian all at once

fdf7b84

Old methods deleted, return to previous commit to find them if needed

Added todo to add multi-area type infilling

a7c5b3e

Param guesses were duplicated

53e3ee7

Some more tidying

05012a2

tbc

Updated documentation

06baaf5

Fixed index page to match style of toolkit. Added pages for new classes/modules

Refactor

471fdb2

Refactored to avoid ugly inheritances from core

File for testing

efaaab7

Some refactoring - multi runs ok

97f7f4b

Changed jacobian to work with different furnessing tolerances in grav…

0e73e51

…ity_function

isaac-tfn and others added 16 commits January 8, 2024 15:43

infill tested on code generated cost matrix

2b64d7d

Black

9a9295e

Fixed tests

3f8207d

Black again

19305ca

isort + black

1c8f965

Fixed docstring

4019264

Fixed refactor in tests

f391a56

Pylint import order fixed

8ea0c7f

Trying to suppress has-type warnings in mypy

febd9bb

Ignore type for strange mypy errors

267f60b

black

08de987

Updated release.md

9943d94

Short docstring for TestUtils

234ce4a

Update src/caf/distribute/gravity_model/multi_area.py

3f1ab26

Co-authored-by: Ben <[email protected]>

typing for plot method

0eccf14

Merge branch 'multi_tld_mvp' of https://github.com/Transport-for-the-…

f64011d

…North/caf.distribute into multi_tld_mvp

BenTaylor-TfN reviewed Jan 9, 2024

View reviewed changes

src/caf/distribute/utils.py Show resolved Hide resolved

tests/gravity_model/test_multi_area.py Show resolved Hide resolved

docs/conf.py Outdated Show resolved Hide resolved

docs/conf.py Outdated Show resolved Hide resolved

isaac-tfn and others added 9 commits January 9, 2024 16:31

Update docs/conf.py

95cf4f8

Co-authored-by: Ben <[email protected]>

Update docs/conf.py

ec135a9

Co-authored-by: Ben <[email protected]>

Merge branch 'multi_tld_mvp' of https://github.com/Transport-for-the-…

cffcf81

…North/caf.distribute into multi_tld_mvp

Fixing review notes

16b7162

Create git blame for isort commits

60d4eac

Forgot to black docs

a3a4057

Added a couple of docstrings

dc9dad7

Imperative mood

e0a3338

PR review changes

e9672b2

BenTaylor-TfN approved these changes Jan 24, 2024

View reviewed changes

BenTaylor-TfN merged commit 33c82e6 into main Jan 24, 2024
14 of 16 checks passed

BenTaylor-TfN deleted the multi_tld_mvp branch January 24, 2024 09:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CAFDI-7 - Create Multi TLD Gravity Model MVP #7

CAFDI-7 - Create Multi TLD Gravity Model MVP #7

wsp-mbuckley commented Sep 7, 2023 •

edited by isaac-tfn

Loading

BenTaylor-TfN commented Sep 20, 2023 •

edited

Loading

codecov bot commented Sep 20, 2023 •

edited

Loading

BenTaylor-TfN left a comment

isaac-tfn commented Jan 17, 2024

CAFDI-7 - Create Multi TLD Gravity Model MVP #7

CAFDI-7 - Create Multi TLD Gravity Model MVP #7

Conversation

wsp-mbuckley commented Sep 7, 2023 • edited by isaac-tfn Loading

Describe Changes

Task Checklist

BenTaylor-TfN commented Sep 20, 2023 • edited Loading

codecov bot commented Sep 20, 2023 • edited Loading

Codecov Report

BenTaylor-TfN left a comment

Choose a reason for hiding this comment

isaac-tfn commented Jan 17, 2024

wsp-mbuckley commented Sep 7, 2023 •

edited by isaac-tfn

Loading

BenTaylor-TfN commented Sep 20, 2023 •

edited

Loading

codecov bot commented Sep 20, 2023 •

edited

Loading