Fix/reset before collect in procedural examples, tests and hl experiment

[maxhuettenrauch]

• I have added the correct label(s) to this Pull Request or linked the relevant issue(s)
• I have provided a description of the changes in this Pull Request
• I have added documentation for my changes
• If applicable, I have added tests to cover my changes.
• I have reformatted the code using poe format
• I have checked style and types with poe lint and poe type-check
• (Optional) I ran tests locally with poe test
  (or a subset of them with poe test-reduced) ,and they pass
• (Optional) I have tested that documentation builds correctly with poe doc-build

Calling collector.collect currently fails if the collector is not reset before collecting. Not sure if there's also a bug in collector where it says ...It has only an effect if n_episode is not None... for the reset_before_collect kwarg as it also happens when collecting n_step.

[maxhuettenrauch]

@bordeauxred, @MischaPanch fyi

[bordeauxred]

@MischaPanch the examples are not covered by the tests. Does it make to include them in cicd or run them once a day (if at least one commit on this day)
or sth?

[MischaPanch]

@MischaPanch the examples are not covered by the tests. Does it make to include them in cicd or run them once a day (if at least one commit on this day) or sth?

Once this is merged, we will follow up and write a script which generates the benchmarks that are currently stored in the examples dir and displayed in the docs. Then we can have a manually triggered GH Action that we trigger on each release. Running examples on each PR/daily is too expensive and unnecessary.

[MischaPanch]

Thanks for spotting!

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.22%. Comparing base (8a0629d) to head (9bac3c7).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1100      +/-   ##
==========================================
+ Coverage   88.21%   88.22%   +0.01%     
==========================================
  Files         100      100              
  Lines        8304     8305       +1     
==========================================
+ Hits         7325     7327       +2     
+ Misses        979      978       -1     

Flag	Coverage Δ
unittests	88.22% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[MischaPanch]

@maxhuettenrauch I thought the sac_with_il seeding problem was fixed now. Is your branch based on the updated master? Or is there some other source of randomness that we're not controlling yet?

[maxhuettenrauch]

@maxhuettenrauch I thought the sac_with_il seeding problem was fixed now. Is your branch based on the updated master? Or is there some other source of randomness that we're not controlling yet?

I doesn't fail on my machine... and it hasn't failed on other PRs, or has it?

[MischaPanch]

@maxhuettenrauch I thought the sac_with_il seeding problem was fixed now. Is your branch based on the updated master? Or is there some other source of randomness that we're not controlling yet?

I doesn't fail on my machine... and it hasn't failed on other PRs, or has it?

It fails occasionally :D, the worst kind of failing. We'll see with you next pushed commit

[maxhuettenrauch]

@maxhuettenrauch I thought the sac_with_il seeding problem was fixed now. Is your branch based on the updated master? Or is there some other source of randomness that we're not controlling yet?

I doesn't fail on my machine... and it hasn't failed on other PRs, or has it?

It fails occasionally :D, the worst kind of failing. We'll see with you next pushed commit

Weird. It's giving me deterministic results on my machine... but maybe another machine produces different numbers on that seed.

[maxhuettenrauch]

Well, I think this still needs some time. The atari_dqn example is broken in another place due to a recent update on master (at least for me).

[MischaPanch]

@maxhuettenrauch the L0 notebook fails for some reason, could you check locally pls?

[maxhuettenrauch]

@maxhuettenrauch the L0 notebook fails for some reason, could you check locally pls?

poe doc-build and poe doc-spellcheck seem to run through on my end. Not sure why it's failing.

[MischaPanch]

@maxhuettenrauch this is finished, right? I'd merge it

Do you know whether it's related (or even solves) #1111 ?

[maxhuettenrauch]

This should (hopefully) cover all instances of the collect before training starts, yes.

Regarding the other issue, I think #1077 broke it.