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

HydroDyn does not allocate A, B, C matrices, leading to segfault in Jacobian linearization outputs #1046

Closed
ebranlard opened this issue Mar 15, 2022 · 3 comments

Comments

@ebranlard
Copy link
Contributor

ebranlard commented Mar 15, 2022

Bug description
Setting LinOutJac=True and LinOutMod=True in the main fst file will result in a segfault because the A, B ,C matrices of hydrodyn are not allocated when the number of states are 0.
This issue comes from the statements if ( p%totalStates == 0 ) return present in HydroDyn.

Potential options

  1. Make sure the Jacobian functions of HydroDyn always allocate the A, B, C, D matrices. We can simply remove the "if" statements, or for some small performance gains, replace the if ( p%totalStates == 0 ) return , with some if ( p%totalStates > 0 ) then ... . This will require a reindentation of the code.

  2. Surround the calls of WrPartialMatrix (in FAST_Lin) with if statement if the matrices are not allocated

  3. Change WrPartialMatrix so that it supports unallocated matrices.

I'd vote for option 1 since I believe other modules like SubDyn always allocate the matrices even if they have a 0 dimension. Let me know your thoughts.

@andrew-platt
Copy link
Collaborator

I vote for option 1 as well.

I have a vague memory of seeing this issue and almost think I have this fixed in a branch, but can't find it now.

@ebranlard
Copy link
Contributor Author

Removing if ( p%totalStates == 0 ) return works fine, should I just do that? That avoids the reindentation of the code.

@andrew-platt
Copy link
Collaborator

andrew-platt commented Mar 15, 2022

That might work. Looking at the allocation on line 2588, the allocation would be 0 by 1 -- we might need to double check that this works with the logic using the dXdu array in the glue code. Explicitly allocating it as 0 by 0 might be safer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants