-
Notifications
You must be signed in to change notification settings - Fork 145
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
Minor changes to SLWF+C logic in parameters.F90 #205
Conversation
…ky situations when combining SCDM with SLWF+C
@@ -2445,7 +2449,7 @@ function get_smearing_index(string, keyword) | |||
|
|||
end function get_smearing_index | |||
|
|||
!=================================================================== | |||
!=================================================================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting that this indentation changed... I wasn't expecting this to happen.
src/parameters.F90
Outdated
if (selective_loc .and. slwf_constrain .and. allocated(proj_site)) & | ||
write (stdout, '(a)') 'No slwf_centres block requested. Desired centres for SLWF same as projection centres' | ||
! Allocate array for constrained centres | ||
call param_get_centre_constraints |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't there be a check that selective_loc
is true? The function internally accesses variables that are now allocated only if (selected_loc)
above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @giovannipizzi, I changed it so it is easier to follow. I don't think I need a check on selective_loc
, since slwf_constrain
is true only if selective_loc
is also true.
… This makes the logic even more trivial
Thanks for clarifying.
make me feel that there is at least one case where the function is called and |
Yes, I think that was the case in my previous commit. It should be OK now, all the tests have passed. |
Seems fine to me. @giovannipizzi - I think your concern has been addressed, and this PR can now be accepted? |
Yes, I had checked it when the tests were still running. Now I merged it! |
Minor changes to SLWF+C logic in parameters.F90
to better handle tricky situations when combining SCDM with SLWF+C