-
Notifications
You must be signed in to change notification settings - Fork 21
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
Replace use of LoopVectorization
in local inertial model stable_timestep
function with Polyester
(reduction
)
#390
Conversation
Polyester v0.7.10 is required for using `reduction` of an already initialized variable.
…mestep` function The internal time step of the local inertial model (`stable_timestep` function) can get zero when `LoopVectorization` is applied (@tturbo) to the for loop of these functions. This issue occured on a virtual machine, Windows 10 Enterprise, with Intel(R) Xeon(R) Gold 6144 CPU (2 processors). This has been fixed by replacing @tturbo by `reduction` of the `Polyester` package.
@JoostBuitink: the Blue Nile model (1d-2d local inertial model) ran fine with these code changes on a WCF node. I think it would be good to also test this for one of your wflow model applications on a WCF node, as part of the review. |
Marked this PR as draft, because I just thought of another solution without the need to replace |
I did a couple of checks with the model that produced errors, and this branch seems to indeed fix it! @shartgring will also do a check, since he just ran into the issue that should be solved by this PR |
I did not get the error myself anymore when redoing my runs so that looks good! At the same time, the Wflow model I recieved from @TBuskop still shows the error. However, that one uses kinematic-wave instead of local-inertial. So, it might be the question whether this PR will solve the error mentioned in the issue. I will elaborate further in the issue itself |
I did check the wflow model of Ted, and the forcing file is not valid. At 2015-11-02 infinite values occur for a couple of grid cells for variable |
Just re-encountered this type of error, so I can also provide the model for further testing (loc iner. 1d floodplains for Rio). We need to merge this branch soon, this error really limits our setups as in most of them floodplains are included. |
@JoostBuitink: could you please add your review? |
Oops, my apologies. I thought I already approved the PR, but it looks like I only left a comment. Should be approved now! |
Issue addressed
Fixes #360
Explanation
The internal time step of the local inertial model (
stable_timestep
function) can get zero whenLoopVectorization
is applied (@tturbo
) to the for loop of these functions. This issue occurred on a virtual machine, Windows 10 Enterprise, with Intel(R) Xeon(R) Gold 6144 CPU (2 processors). This has been fixed by replacing@tturbo
withreduction
of thePolyester
package.Checklist
master
Additional Notes (optional)
Add any additional notes or information that may be helpful.