Transport-for-the-North / caf.distribute

https://cafdistribute.readthedocs.io/en/stable/
Other
0 stars 1 forks source link

Refactor gravity model #6

Closed BenTaylor-TfN closed 1 year ago

BenTaylor-TfN commented 1 year ago

Describe Changes

Refactoring the single TLD gravity model to make it more intuitive to use.

Creating a front end with different, simpler run, run_percieved, and calibrate functions.

Task Checklist

BenTaylor-TfN commented 1 year ago

@wsp-mbuckley, any thoughts on this refactored gravity model?

As an overview the front end now looks something like:

        gm = SingleAreaGravityModelCalibrator(
            row_targets=row_targets,
            col_targets=col_targets,
            cost_function=cost_function,
            cost_matrix=cost_matrix,
        )

Then the model can be run with either gm.run(), or gm.run_with_perceived_factors():

        gm.run_with_perceived_factors(
            cost_params=cost_params,
            running_log_path=running_log_path,
            target_cost_distribution=target_cost_distribution,
            max_iters=furness_max_iters,
            tol=furness_tol,
        )

calibration working in a similar way with gm.calibrate(), or gm.calibarte_with_perceived_factors(), taking many more arguments.

I've also updated caf.toolkit to implement a CostDistribution class: https://github.com/Transport-for-the-North/caf.toolkit/pull/27 This is used anywhere a cost distribution was in the past.

Finally, any of the above 4 function calls all return a GravityModelResults dataclass storing all the results, defined here: https://github.com/Transport-for-the-North/caf.distribute/blob/047c847d2736f883f58fba01dc1a440078f4cd3b/src/caf/distribute/gravity_model/core.py#L35

This only works for single area at the moment. If you're happy with this general interface, I'll finalise the tests and move on to implementing a multi-TLD equivalent!

wsp-mbuckley commented 1 year ago

@wsp-mbuckley, any thoughts on this refactored gravity model?

As an overview the front end now looks something like:

        gm = SingleAreaGravityModelCalibrator(
            row_targets=row_targets,
            col_targets=col_targets,
            cost_function=cost_function,
            cost_matrix=cost_matrix,
        )

Then the model can be run with either gm.run(), or gm.run_with_perceived_factors():

        gm.run_with_perceived_factors(
            cost_params=cost_params,
            running_log_path=running_log_path,
            target_cost_distribution=target_cost_distribution,
            max_iters=furness_max_iters,
            tol=furness_tol,
        )

calibration working in a similar way with gm.calibrate(), or gm.calibarte_with_perceived_factors(), taking many more arguments.

I've also updated caf.toolkit to implement a CostDistribution class: Transport-for-the-North/caf.toolkit#27 This is used anywhere a cost distribution was in the past.

Finally, any of the above 4 function calls all return a GravityModelResults dataclass storing all the results, defined here:

https://github.com/Transport-for-the-North/caf.distribute/blob/047c847d2736f883f58fba01dc1a440078f4cd3b/src/caf/distribute/gravity_model/core.py#L35

This only works for single area at the moment. If you're happy with this general interface, I'll finalise the tests and move on to implementing a multi-TLD equivalent!

I think it makes sense having separate functions because I think by default people should use run or calibrate and should only use the perceived methods if they know what they're doing

BenTaylor-TfN commented 1 year ago

Trying to wrap this up so we can move onto Multi-TLD Gravity model ASAP.

Note that tests and linting will fail here until https://github.com/Transport-for-the-North/caf.toolkit/pull/27 is complete. However, these are passing locally with that branch of toolkit.

Test Value Updates

Some test values have been updated in this PR, detail below. Where test inputs and outputs were updated, these were checked locally to produce the same results as the main branch.

Simple Tanner

This test was returning 0 for convergence so wasn't particularly useful. Furthermore, the updates in this code meant that some remedies were applied for failing tests, meaning an actual convergence was found! Some fixes were made to allow tests to be run with init_params other than the default values, meaning these tests now achieve a testable convergence.

Realistic Calibrate and Run (with perceived factors)

There was a bug in the code on main, meaning perceived factors weren't ever being applied. I implemented a quick and dirty fix on main the test what the result would be if factors were applied. The new test values are validated against this fix.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage is 77.77% of modified lines.

Files Changed Coverage
src/caf/distribute/cost_functions.py 33.33%
src/caf/distribute/gravity_model/single_area.py 35.29%
src/caf/distribute/gravity_model/core.py 83.43%
src/caf/distribute/gravity_model/__init__.py 100.00%

:loudspeaker: Thoughts on this report? Let us know!.