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

Fplan 32 unit tests #34

Merged
merged 7 commits into from
Feb 14, 2024
Merged

Fplan 32 unit tests #34

merged 7 commits into from
Feb 14, 2024

Conversation

cmovic
Copy link
Contributor

@cmovic cmovic commented Jan 30, 2024

A test framework strawman using pytest. The tests are just dummies so comments on the framework are welcome. If positive, I'll flesh out the tests.

Also I wasn't sure if I wanted to test loads of the example files or keep some separate for ease of maintenance. For now there's one of each and some notes about pros and cons.

@cmovic
Copy link
Contributor Author

cmovic commented Jan 30, 2024

These changes are for issue #32 - I don't know how to link them (or if I should).

@wscott
Copy link
Owner

wscott commented Jan 30, 2024

This looks useful.

So I was thinking of using some focused examples like this one:

returns = 0
inflation = 0
startage = 30
endage = 80

[taxes]
taxrates = [[0, 20]]
stded = 100_000            # standard deduction
cg_tax = 0 # doesn't work

[aftertax]
bal = 500_000
basis = 500_000

[roth]
bal = 9_000_000
contributions = [[30, 9_000_000]]

Where we predict the answer will be exactly 100k spending every year and then verify that happens.

And others where we can verify the tax calculations.

@cmovic
Copy link
Contributor Author

cmovic commented Jan 30, 2024

Thanks for the feedback. I'll flesh out the load_file tests, then try creating a solver test based on the above inputs.

@cmovic
Copy link
Contributor Author

cmovic commented Jan 31, 2024

While building a solve() test I ran into a snag where globals declared in the test code are not visible in the fplan functions being tested. I think the best way forward is to refactor the globals as follows:

Move these constants into the Data class:

    global vper, n0, n1
    vper = 4        # variables per year (savings, ira, roth, ira2roth)
    n1 = 2          # before-retire years start here
    n0 = n1+S.workyr*vper   # post-retirement years start here

Then, pass the configuration data S to solve(), print_ascii, and print_csv.

I'll try to make this refactoring as minimally invasive as possible.

@cmovic
Copy link
Contributor Author

cmovic commented Jan 31, 2024

Solver tests added. I'm really curious if others get the same results as I do. My expectation is that we'll see small numerical differences from different versions of scipy.

assert cfg.roth['contributions'] == [[54, 20000], [55, 20000]]

# TODO Determine if this is fragile on other python versions. FP compares like this give me the willies.
assert cfg.income == [0, 0, 0, 0, 0, 47802.892452600514, 48806.75319410512, 49831.69501118133, 50878.16060641613, 51946.60197915086, 53037.480620713024, 54151.26771374799, 55288.4443357367, 56449.50166678716, 57634.94120178968, 58845.27496702727, 60081.02574133483, 61342.72728190286, 62630.924554822814, 63946.17397047408, 65289.043623854035, 66660.11353995497, 68059.975924294, 69489.23541870418, 70948.50936249696, 72438.42805910938, 73959.63504835068, 75512.78738436603, 77098.55591943773, 78717.6255937459, 80370.69573121457, 82058.48034157006, 83781.70842874303, 85541.1243057466, 87337.48791616729]
Copy link
Owner

Choose a reason for hiding this comment

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

I can't imagine this will work. Even switching compiler knobs on x86 will break this even with the same python versions. I would suggest rounding all the numbers of the nearest 100 and then comparing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Converting these checks to use math.isclose() similar to what I just pushed to test_sample_solve.py. Note: rounding doesn't work for FP compares because a single bit difference can cause the rounds to go in opposite directions. However, math.isclose(x, y, abs_tol=100) accomplishes the desired result.

@cmovic
Copy link
Contributor Author

cmovic commented Feb 3, 2024 via email

@cmovic
Copy link
Contributor Author

cmovic commented Feb 4, 2024 via email

@wscott
Copy link
Owner

wscott commented Feb 14, 2024

FWIW rounding doesn’t work because you can still have 1 bit
differences that round up or down.

That just isn't true. A single-bit error can mean the difference between 3.00000001 & 2.99999999, but both would round to the same number.

The isclose() approach is fine as long as the epsilon is way above the error from the solver. Most of these values are money and just getting within $1 is perfectly fine.

Also, it is straightforward to generate an input where spending for some years are unconstrained and the answers can vary a lot. For example, if it may be that the years right after retirement or before social security set the max spending. So later years have lots of possible solutions. For these, the solution can move around.

@wscott wscott merged commit af361b4 into wscott:master Feb 14, 2024
@cmovic
Copy link
Contributor Author

cmovic commented Feb 14, 2024

Hmm. I wasn't done with my changes. I'll create another fork and finish up.

As for rounding, a single bit will still cause differences. Your example of 3.000000001 and 2.99999999 both rounded to nearest unit both result in 3.0, but 2.500000001 and 2.499999999 will unit round to 3 and 2 respectively. I think an edge case like this exists for any floating point rounding.

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.

2 participants