search

DynareJulia / FastLapackInterface.jl

MIT License
32 stars 9 forks source link

Resize workspace to smaller system #22

Closed jebej closed 2 years ago

jebej commented 2 years ago

Hello, It seems that at the moment, for the eigen workspaces, a resize is only triggered if the size of the workspace is too small, but not if it is too large: https://github.com/DynareJulia/FastLapackInterface.jl/blob/e0d766127dec795188402e2d7e75926f73039538/src/eigen.jl#L379

The function therefore returns invalid (too large) arrays for eigenvector and for eigenvalues.

louisponet commented 2 years ago

Dear Jebej,

Thank you, you are correct. I will change the behavior so that the right arrays are returned.

louisponet commented 2 years ago

I think #23 solved this issue, let me know if you agree! Thanks again for your input it's very valuable

jebej commented 2 years ago

I think #25 is still a better fix than the current iteration of #23

louisponet commented 2 years ago

Could you be more specific as to why? I'm trying to avoid unnecessary LAPACK calls.

jebej commented 2 years ago

I left a comment in #23

louisponet commented 2 years ago

Sorry didn't see that one. My notifications are a bit iffy. It's a fair comment though, I'll discuss with @MichelJuillard what he thinks.

MichelJuillard commented 2 years ago

I would go with resizing instead of views, so #25 rather than #23, but I wouldn't resize down the Lapack workspace the way #25 is currently doing.

louisponet commented 2 years ago

I have removed the views in the last #23 iteration, i.e. now the actual storage for the eigenvalues and vectors are changed/resized, and returned as normal arrays, whereas the work buffers are kept at the larger size.