Add checkpointing to metagraph build

[danieldanciu]

As discussed via chat, there are multiple checkpoints after each major operation: generate kmers, generate reverse complements, splitting into chunks, generate dummy1, concatenate chunks, generate dummy2..k.

The client interface exposes only 2 phases:

• phase 1 builds everything up to and including dummy k-mers
• phase 2 finishes the build

I tested by inserting exit statements in the code after each phase and making sure the build is successful on resume.
I also added functional tests for building in phases.

[karasikov]

Could you add a test where it builds one graph, kills the process, so it fails to build to the end.
Then it does the same thing for a different graph. Then restarts building both graphs in parallel, and checks that both graphs are correctly generated.

So we check that there are no collisions here and you can correctly generate multiple graphs in parallel on the same machine.

[danieldanciu]

I can add a test that builds 2 graphs in parallel (using phases), but I won't kill the process as that causes the test to be undeterministic.

…

On Tue, 8 Sep 2020 at 19:31, Mikhail Karasikov ***@***.***> wrote: Could you add a test where it builds one graph, kills the process, so it fails to build to the end. Then it does the same thing for a different graph. Then restarts building both graphs in parallel, and checks that both graphs are correctly generated. So we check that there are no collisions here and you can correctly generate multiple graphs in parallel on the same machine. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#197 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOYQLCNGGYLC7VF3SD5WZLSEZS75ANCNFSM4RAHV52Q> .

[danieldanciu]

Ok, I added a test that builds 2 graphs at the same time. First it builds phase 1 for both graphs, then it builds phase2 for both and asserts that the graphs are correctly built.

…

On Tue, 8 Sep 2020 at 19:37, Daniel Danciu ***@***.***> wrote: I can add a test that builds 2 graphs in parallel (using phases), but I won't kill the process as that causes the test to be undeterministic. On Tue, 8 Sep 2020 at 19:31, Mikhail Karasikov ***@***.***> wrote: > Could you add a test where it builds one graph, kills the process, so it > fails to build to the end. > Then it does the same thing for a different graph. Then restarts building > both graphs in parallel, and checks that both graphs are correctly > generated. > > So we check that there are no collisions here and you can correctly > generate multiple graphs in parallel on the same machine. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#197 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAOYQLCNGGYLC7VF3SD5WZLSEZS75ANCNFSM4RAHV52Q> > . >

[karasikov]

Write the phase too. Otherwise, how do you know which phase this checkpoint corresponds to?

If you assume that different phases have different checkpoints, like 1: (1,2,3), 2: (4,5,6), then make the interface not allow setting checkpoint = 0 for the second phase.

[danieldanciu]

Like you said, the phases correspond to different checkpoints, so writing the phase is not useful - it just tells the program up to which checkpoint it should go before stopping.
So if the users enters phase=1, we might make it up to checkpoint 2, and then resume the operation from checkpoint 2 and finalize the phase.

Setting checkpoint_=0 for phase_=2 is perfectly valid. One tells us how far we got with the computation, the other how far we wish to go. The other way around, setting checkpoint_=5 and phase_=1 could be invalid, but in our case phase_=1 corresponds to having all checkpoints done, so any combination of valid checkpoint (0..5) and phase values (1..2) is fine.

[karasikov]

looks like a variable name commented out

Suggested change

	real_split_by_W, false /* remove sources */
	real_split_by_W, false // remove sources

[danieldanciu]

That's the standard way of commenting parameters represented by constants. You can only use // here because the param happens to be at the end of the line.

[karasikov]

It's better to have non-deterministic tests than no tests.
The expected result is well determined. The second run of metagraph started after a previous run that was killed, must construct a valid graph.
So it's a perfectly defined test, isn't it?

I can add a test that builds 2 graphs in parallel (using phases), but I won't kill the process as that causes the test to be undeterministic.
…
On Tue, 8 Sep 2020 at 19:31, Mikhail Karasikov @.***> wrote: Could you add a test where it builds one graph, kills the process, so it fails to build to the end. Then it does the same thing for a different graph. Then restarts building both graphs in parallel, and checks that both graphs are correctly generated. So we check that there are no collisions here and you can correctly generate multiple graphs in parallel on the same machine. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#197 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOYQLCNGGYLC7VF3SD5WZLSEZS75ANCNFSM4RAHV52Q .