Allow seed to be set for serialized pipeline

[szalpal]

Similarly to other Pipeline params (refer the other ctor), this PR allows seed to be set for ctor with serialized pipeline.

Few comments required:

I'm aware, that -1 might be legitimate seed value, thus prolly shouldn't be a default value, but not in our code:

    if (seed < 0) {
      using Clock = std::chrono::high_resolution_clock;
      seed = Clock::now().time_since_epoch().count();
    }

Without optional there's no clean way to handle default seed in this case. I could go with int64_t* seed = nullptr, but alongside the rest of parameters, it would be obscure. using optional = std::pair<T, bool> also looks questinably compliated. Plus, we go with -1 as default seed anyway: in the other ctor.

I have an impression, that the seed was used incorrectly. Let's say, that -1 is passed to the Init(...). original_seed_ is set to -1. Then, in the snippet I pasted above, seed is set according to clock. Finally, when somebody serializes the pipeline (SerializeToProtobuf), the -1 is set as the original_seed_, instead of the clock-wise value assigned before. It might be intentional though, please double-check.

Signed-off-by: Michał Szołucha szal@szal-mbp.local

[szalpal]

!build

[dali-automaton]

CI MESSAGE: [1220604]: BUILD STARTED

[dali-automaton]

CI MESSAGE: [1220604]: BUILD FAILED

[JanuszL]

Now any code that uses the old Pipeline(const string &serialized_pipe signature is brokenand no one knows why.

[JanuszL]

How about moving it to the last position?

[szalpal]

True, changed

[klecki]

Now it is inconsistent in a different way with other constructors.

[JanuszL]

I have an impression, that the seed was used incorrectly. Let's say, that -1 is passed to the Init(...). original_seed_ is set to -1. Then, in the snippet I pasted above, seed is set according to clock. Finally, when somebody serializes the pipeline (SerializeToProtobuf), the -1 is set as the original_seed_, instead of the clock-wise value assigned before. It might be intentional though, please double-check.

seed (int, optional, default = -1) – Seed used for random number generation. Leaving the default value for this parameter results in random seed.
So user has been warned. We also use std::seed_seq ss{seed}; to generate per operator seeds and it doesn't accept negative values AFAIK.

[JanuszL]

I have an impression, that the seed was used incorrectly. Let's say, that -1 is passed to the Init(...). original_seed_ is set to -1. Then, in the snippet I pasted above, seed is set according to clock. Finally, when somebody serializes the pipeline (SerializeToProtobuf), the -1 is set as the original_seed_, instead of the clock-wise value assigned before. It might be intentional though, please double-check.

seed (int, optional, default = -1) – Seed used for random number generation. Leaving the default value for this parameter results in random seed.
So user has been warned. We also use std::seed_seq ss{seed}; to generate per operator seeds and it doesn't accept negative values AFAIK.

That is true that others FWs accepts also negative values, but I don't know if this is used by anyone. If we want to truly support this we need to rework std::seed_seq ss{seed};, I don't have any strong opinion.

[szalpal]

!build

[dali-automaton]

CI MESSAGE: [1221209]: BUILD STARTED

[dali-automaton]

CI MESSAGE: [1221209]: BUILD FAILED

[dali-automaton]

CI MESSAGE: [1221209]: BUILD PASSED