ChrisCScott / forecaster

A personal finances forecasting tool for Canadian retirement planning
Other
1 stars 2 forks source link

Graph-based TransactionStrategy #68

Closed ChrisCScott closed 5 years ago

ChrisCScott commented 5 years ago

This PR reimplements TransactionStrategy. It now uses a networkx-based graph algorithm (namely network flow) rather than the former tree-based algorithm. This ramps up the computational complexity of the class, but it also provides superior results.

Chief of these improved results is that higher-level nodes' constraints will be prioritized over lower-level nodes. For example, if there's a possible allocation that allows a top-level weighted node to distribute flow between children proportionately to their weights, such an allocation will always be adopted over an allocation that doesn't. Under the old algorithm, this wasn't guaranteed, and in fact it was straightforward to wind up in a situation where weighted nodes' children received disproportionate allocations even if a proportionate allocation was available.

This PR adds extensive tests for TransactionStrategy and supporting code, including for the various scenarios described above. It also adds docstrings for several existing methods (some of which are related only peripherally to TransactionStrategy, such as LinkedLimitAccount). Finally, CircleCI and VSCode settings have been updated.

This PR is far too large. It should've been submitted a dozen or so commits ago. But rather than roll back the clock, we'll just aim to issue PRs more frequently and for smaller changes going forward.

This PR closes #66, closes #58,

This improves on an PR relating to TransactionStrategy, which (should have) closed #43 and closed #51.