Add IRR to cashloan and migrate functions to lib_financial.h#1223
Conversation
Also clean up IRR code
Declare all functions in lib_financial.h Clean up comments and update URLs to code references
sjanzou
left a comment
There was a problem hiding this comment.
For Mac and Linux GitHub Actions to pass, please add the following to the lib_financial.cpp file for DBL_MAX to be defined:
DBL_MAX is defined in float.h and was previously included at the top of each financial compute module as
#ifndef WIN32
#include <float.h>
#endif
| } | ||
|
|
||
| double scale_factor = irr_scale_factor(cf_vector, count); | ||
| double residual = DBL_MAX; |
There was a problem hiding this comment.
DBL_MAX was not defined and is causing the Linux and MacOS tests to fail.
DBL_MAX is defined in float.h and was previously included at the top of each financial compute module as
#ifndef WIN32
#include <float.h>
#endif
@sjanzou Thanks for finding the cause of those tests failing. |
brtietz
left a comment
There was a problem hiding this comment.
Refactoring looks good! One request for a clarifying comment. Thanks!
| #endif | ||
|
|
||
| double libfin::min(double a, double b) | ||
| { // handle NaN |
There was a problem hiding this comment.
Can you or @sjanzou expand upon this comment to include why 0 is the correct answer when one of the inputs is NaN? My hunch based on reading the code is to make the cashflow readable, but it would be good to document that if true so future developers know whether to use this or std::min.
There was a problem hiding this comment.
@brtietz , yes - to make the cashflow readable and that std::min and max have screwy results based on if a NaN is the first element in the comparison - see https://stackoverflow.com/questions/72296440/how-to-prevent-stdmin-and-max-to-return-nan-if-the-first-element-of-the-array
sjanzou
left a comment
There was a problem hiding this comment.
Working for me and all platforms and GitHub Actions...
Thanks for taking the plunge into modularization!
Fix NatLabRockies/SAM#638
1. Add IRR output to cashloan
2. Move financial model functions to lib_financial for all financial models
depreciation_sched_custom()using namespace libfinfrom cmod* Note that Third Party Ownership - Host model has payback cashflows but does not use the payback() function to calculate payback period.
Question
That there are special
min()andmax()functions defined in lib_financial. I changed all calls tomin()andmax()in the financial cmods tolibfin::min()andlibfin::max()to avoid conflict, but should any of those bestd::min()orstd::max()instead?