rhaiscript / rhai-rand

Random numbers generation package for Rhai.
https://crates.io/crates/rhai-rand
Apache License 2.0
5 stars 2 forks source link

Full optimization optimizes out randomization #5

Closed tprodanov closed 8 months ago

tprodanov commented 8 months ago

As in the title: if I set engine.set_optimization_level(OptimizationLevel::Full) and then in the embedded scripts I have rand(1, 10), the engine will remove function call and will always produce the same random value in the for loop/function call.

For example, in Rhai:

fn test_fn() {
    for i in 0..10 {
        print(`Random value = ${rand(1, 10)}`);
    }
}

will produce

Random value = 1
Random value = 5
Random value = 10
Random value = 9
Random value = 4
Random value = 10
Random value = 4
Random value = 10
Random value = 10
Random value = 7

at default optimization level, and

Random value = 3
Random value = 3
Random value = 3
Random value = 3
Random value = 3
Random value = 3
Random value = 3
Random value = 3
Random value = 3
Random value = 3

at full optimization level.

schungx commented 8 months ago

That is by design.

https://rhai.rs/book/engine/optimize/eager.html

https://rhai.rs/book/engine/optimize/side-effects.html#admonition-rule-of-thumb

tprodanov commented 8 months ago

I understand the concept of pure functions and the reasoning behind this problem, but I would not call it "by design". At best, it is a limitation of the optimizer, not a feature. In any case, would it be possible to add a big warning section to the rhai-rand documentation and book section? Ideally, would be possible to add optimizer hints that the function is volatile in the future versions?

schungx commented 8 months ago

It probably is possible to mark the function as volatile... But why can't the script be compiled before the library is loaded into the engine?

If that's inconvenient, it still is possible to keep two Engine's, one for compilation and one for execution.

tprodanov commented 8 months ago

I agree, it is totally possible, and now I did that, but it is really not obvious and a bit error-prone.

schungx commented 8 months ago

True in that respect.

I'll need to think a bit to decide whether it is good to introduce yet another flag to plugin functions, as there are already many.

tprodanov commented 8 months ago

But would not such flag useful for many use cases? Such as functions that should not be called during compilation, or get current time, as in the docs?

In any case, thank you for answering!

schungx commented 8 months ago

OK, you talked me into it.

I'll add volatile marker for plugin modules to the next version. Then this will be solved.