Ah…but it doesn’t “already work”. We need to either correct the NaNs (which we can do by replacing them with -Inf because we know that -Inf is the only value that creates them in the first place), or by making all the non-zeros some very small nonzero amount (exp(-500)), which will prevent -Inf from being multiplied by zero.
Without one fix or the other, the repo gives bad results. What’s nice about there being a function is that we can decide which fix to implement in one place, call that function in place of interp1, and be done with it.
As an aside, I’ve found some other places where sparse matrices are being created that are actually full. When the dust has settled, I’ll track those down and create a pull request.
1 Like
@aledinola
I just cosmetically overhauled the sparse howards commands with grid interpolation layer to be a bit more in toolkit style code. I’m letting you know in case anything broke, I ran tests on Aiyagari model with and without endogenous labour and they seemed fine.
Main change was that you are not allowed functions that are defined in the foot of a code. So nothing like,
function cumsumresult=cumulativesum(myVector)
cumsumresult=0
for vv=1:length(myVector)
cumsumresult=simpleSum(cumsumresult,myVector(vv));
end
end
function C=simpleSum(A,B)
C=A+B;
end
If this thing ‘simpleSum’ is important enough to warrant a function then it needs to be a standalone file so that all the different parts of VFI Toolkit can call it and get the exact same behaviour. If it is not important enough for that, just hardcode it.
I know that lot’s of people follow this kind of ‘subfunction’ approach of putting functions in the foot of other functions. But I think from the toolkit perspective it is bad behaviour as these kind of subfunctions should be shared and common across everything. Partly just as it makes it easier to follow what is going on, and partly as that way if some behaviour needs changing you edit the one version and it fixes all of the uses.
[The other change was that you dealt with turning ReturnFnParamsVec into a Cell before all the loops, and checking the size of it. I’ve moved it back into the loop (as it is done inside the CreateReturnFnMatrix commands). Really how you did it is something the toolkit should do too, but it has to be something that is rolled out to all the toolkit or none. Having different commands in the toolkit behave in different ways is a recipe for the whole thing becoming indecipherable. There are nowadays a few hundred value fn iteration commands, so they have to keep the same setup common across them or it will all become impossible to manage.]
1 Like
Fully agree with you, thanks for the inprovements.
The only point I’m not sure is this one:
Since ReturnFnParamsVec is loop invariant, isn’t it better for performance to move it before the loops?
Yes it is better. But this is not presently how CreateReturnFnMatrix commands work. At some point they should be converted to take ReturnFnParamsCell as input instead of ReturnFnParamsVec. (And the checks inside CreateReturnFnMatrix should be moved to way earlier and done once.) While this change is trivial it will also need to be done in say 200ish places. I will put it on the to-do list, but is likely to take some time before I am game to face this one.
1 Like
Absolutely, it makes sense. There is alsways a trade-off between performance and code maintenability. The way things are now makes the code much easier to maintain: my solution was bit “quick and dirty”.
Going forward, your proposal is the best. Have you tried using claude or chatgpt for these easy but time-consuming tasks? With LLM you can upload say 10 matlab files and ask the ai to do the work for you. Then you have to check but nowadays they tend to be accurate IF the task is not difficult and repetitive
For matlab is important to save this prompt in the persistent memory of your AI:
When coding in Matlab, I often use parallelization on the gpu with gpuArray and arrayfun. When possible, prefer vectorized code to loop-based code.
1 Like
I will plan to move everything over to ReturnFnParamsCell rather than ReturnFnParamsVec in late February. That way I will make the change when I am not going to be going anywhere for the next few weeks and so I have plenty of time to track down and clean up any issues that arise. [I am away for two weeks in mid Feb, so now is not a good time to be doing it]
2 Likes
To be more precise, I found useful to add this piece to the persistent memory of your LLM (I use ChatGPT and Claude):
Matlab: I also use parallelization on the GPU (gpuArray, arrayfun, etc.). Prefer vectorized code when possible. Always use implicit expansion instead of repmat or bsxfun.
It’s important to mention implicit expansion since LLM trained on old Matlab code might give you inefficient code based on useless tiling of arrays. Typical example for the budget constraint of an income fluctuation model:
a_grid = linspace(0,20,n_a)'; % column vector
% result is matrix (a',a)
consumption = (1+r)*a_grid' + wage - a_grid;
More efficient than the old fashioned
a_grid = linspace(0,20,n_a)'; % column vector
% consumption is matrix (a',a)
consumption = (1+r)*repmat(a_grid',n_a,1) + wage - repmat(a_grid,1,n_a);