search

shadow / tornettools

A tool to generate realistic private Tor network models, run them in Shadow, and analyze the results.
Other
34 stars 15 forks source link

Update run_all_steps_output #86

Closed sporksmith closed 2 years ago

sporksmith commented 2 years ago

Using data from last run on main: https://github.com/shadow/tornettools/actions/runs/2437635597

sporksmith commented 2 years ago

I was expecting that the "golden" and "current" plots in this PR's run would be identical now that we've fixed more determinism issues in shadow. Unfortunately that doesn't seem to be the case. (See "plots" artifact at https://github.com/shadow/tornettools/actions/runs/2632801430)

stevenengler commented 2 years ago

I'm not clear on what this PR is for, and why it needs to be updated. And is the golden version the version at https://github.com/shadow/tornettools/tree/main/.github/workflows/run_all_steps_output? If so, I think I would expect it to have different results since it was last updated 5 months ago and used a different shadow version?

sporksmith commented 2 years ago

I'm not clear on what this PR is for, and why it needs to be updated.

We haven't updated the golden data for a while, and in particular not since recently fixing some determinism bugs in shadow and updating this pipeline to that version of Shadow.

My hope was that now that we'd fixed those bugs, runs for PRs that don't change behavior at all would have identical output. i.e. after updating the golden output here, plots for subsequent runs of "no-op" PRs would have identical "golden" and "current" lines.

Unfortunately that doesn't seem to be the case, but it still doesn't hurt to "bump" to data produced from a more recent version and CI configuration.

And is the golden version the version at https://github.com/shadow/tornettools/tree/main/.github/workflows/run_all_steps_output? If so, I think I would expect it to have different results since it was last updated 5 months ago and used a different shadow version?

The "golden" line is the updated version of workflows/run_all_steps_output from this PR, which was taken from the most recent run on the main branch: https://github.com/shadow/tornettools/actions/runs/2437635597

stevenengler commented 2 years ago

My hope was that now that we'd fixed those bugs, runs for PRs that don't change behavior at all would have identical output. i.e. after updating the golden output here, plots for subsequent runs of "no-op" PRs would have identical "golden" and "current" lines.

Unfortunately that doesn't seem to be the case, but it still doesn't hurt to "bump" to data produced from a more recent version and CI configuration.

The tornettools run_all_steps.yml stage and simulate steps are using randomized seeds, so maybe that's why? The benchmark repo produces deterministic results for a 5% network, so I would think that this sim should also.

The "golden" line is the updated version of workflows/run_all_steps_output from this PR, which was taken from the most recent run on the main branch: https://github.com/shadow/tornettools/actions/runs/2437635597

I think I'm even more confused now, but that's alright :)

sporksmith commented 2 years ago

The tornettools run_all_steps.yml stage and simulate steps are using randomized seeds

It looks like the seeds are hard-coded to 1, or am I missing something?

https://github.com/shadow/tornettools/blob/5ee84cef2690143f6adf2667d1db9fd5f7d7d3a4/.github/workflows/run_all_steps.yml#L211

https://github.com/shadow/tornettools/blob/5ee84cef2690143f6adf2667d1db9fd5f7d7d3a4/.github/workflows/run_all_steps.yml#L229

stevenengler commented 2 years ago

The tornettools run_all_steps.yml stage and simulate steps are using randomized seeds

It looks like the seeds are hard-coded to 1, or am I missing something?

https://github.com/shadow/tornettools/blob/5ee84cef2690143f6adf2667d1db9fd5f7d7d3a4/.github/workflows/run_all_steps.yml#L211

This is for the "generate" step, not the "stage" step. I'm not sure that the staging process is deterministic for randomized seeds.

https://github.com/shadow/tornettools/blob/5ee84cef2690143f6adf2667d1db9fd5f7d7d3a4/.github/workflows/run_all_steps.yml#L229

This sets a shadow seed, but not a tornettools seed. But this one probably doesn't matter since the simulate step doesn't do much.

Edit: Actually I'm not sure that the "stage" step would be deterministic even with a fixed seed since tornettools uses a work pool, and it processes files in a non-deterministic order (it uses the order returned by the filesystem).

sporksmith commented 2 years ago

This is for the "generate" step, not the "stage" step. I'm not sure that the staging process is deterministic for randomized seeds.

Oops, right. I thought that it defaulted to hard-coded seed in that case, but it looks like it's truly random.

https://github.com/shadow/tornettools/blob/5ee84cef2690143f6adf2667d1db9fd5f7d7d3a4/tornettools/tornettools#L596

This sets a shadow seed, but not a tornettools seed. But this one probably doesn't matter since the simulate step doesn't do much.

Oops, yeah.

I'm going to try setting the tornettools seed at each step. This CI would be a lot more useful if it were fully deterministic.

Thanks!

sporksmith commented 2 years ago

Obsoleted by #87