ported Event example from old SimJulia doc

[asmirnov69]

Fixes #106

[Krastanov]

Thank you! There is a small typo in a test we merged on master while working on something. I will fix that unrelated typo and merge your PR over the weekend.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (34ed8dd) 97.15% compared to head (9771cf6) 97.15%.
Report is 4 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #107   +/-   ##
=======================================
  Coverage   97.15%   97.15%           
=======================================
  Files          12       12           
  Lines         316      316           
=======================================
  Hits          307      307           
  Misses          9        9           

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

[Krastanov]

Hi, Andrei!

Concerning CI: The CI-nightly test is not related to your changes, do not worry about it. Could you briefly mention the documentation update in the CHANGELOG file? The performance tracking is not actually set up, so also do not worry about it.

I pushed to your branch to retrigger the CI -- you might need to pull or force push to make sure future changes of yours are made accessible here. I also noticed you are working directly on your master branch -- this is perfectly workable, but it can cause you issues if you start working on multiple changes. I would generally suggest always creating separate branches with something like git checkout -b NEW_BRANCH_NAME.

The docs you added are in the form of a literal code block, so they do not actually get tested for correctness. In order to ensure that the docs and the code do not diverge, we try to make sure all example code in the docs is always executed. That can be done in various ways. E.g. we use jldoctest or @example markers. Look through the rest of the docs for how it is done or check out this list of options: https://documenter.juliadocs.org/stable/man/syntax/

I think for your situation just a jldoctest will be enough.

This can be a bit finicky to set up, so do not hesitate to message if something is unclear.

Could you also remove the script from the examples folder? Currently that folder is a bit of an untested outdated mess and it would probably be best to rework it to make sure it is part of the visible documentation.

Thanks for helping with all of this, it is really valuable!

[Krastanov]

I added issue #111 to track the examples folder cleanup that I mentioned in the previous comment.

[asmirnov69]

• CHANGELOG.md updated - I bumped version string in there, not sure is it right way to do that
• school.jl example removed
• jldoctest used to mark school code example in events.md

Note that doctest is disabled in docs/make.jl. Also, it seems that Revise import is not necessary. I decided not to struggle with fixing of existing jldoctest errors at this point.

I agree that it does make sense to work on separate branch in fork as additional measure to keep thing clear for everyone.
If you can merge this changes and close PR I would open another PR for more docs cleanup work - this time on separate branch

[Krastanov]

I do not believe there are pre-existing doctest errors. The doctests are run when we run the tests (from the test/test_doctests.jl file). The Revise in the docs/make.jl is there for when we make the docs locally as it significantly helps with iterative updates to the docs.

The example still needs some more work as it is not currently verifying the output (which is the reason for breakages in the CI). Could you flesh it out? Here are some guidelines on how to set it up https://documenter.juliadocs.org/stable/man/doctests/

For locally running the docs you can use:

❯ julia --project=docs -e '
              using Pkg
              Pkg.develop(PackageSpec(path=pwd()))
              Pkg.instantiate()';
  julia --project=docs -i docs/make.jl

It is the -i in the last command that lets you run in interactive mode. Then to rebuild the docs without compiling a ton of stuff from scratch you can do include("./docs/make.jl")

[asmirnov69]

can you suggest how to properly run test/test_doctests.jl from shell?
I was trying to use such command (from project repo top-level):

julia --project=. test/test_doctests.jl

I think it fails mostly because some output strings are not prepended with ConcurrentSim.

Anyway, I put output into school example and commit/push. I think output should match and CI will run. I guess it requires your approval to go through.

Do you know is it possible to see what exactly CI run is doing? I found yml files in .github but where detailed logs are stored?

[Krastanov]

To find the logs from CI, go to the header of this PR and click on "checks"

Then click on CI and find the failing CI run. Here is direct link to the latest run: https://github.com/JuliaDynamics/ConcurrentSim.jl/actions/runs/7834706506/job/21381106568

I think you are simply missing a space in the line # output

[Krastanov]

ah, you fixed it just as I was sending the comment :D

[Krastanov]

for running the tests, it is easiest just to do it from the REPL with ] test ResumableFunctions. There is a way to select by test name, but I have not really used it much -- something to do with Pkg.test("ResumableFunctions", args=...)

[asmirnov69]

for running the tests, it is easiest just to do it from the REPL with ] test ResumableFunctions. There is a way to select by test name, but I have not really used it much -- something to do with Pkg.test("ResumableFunctions", args=...)

OK, i will follow this advice.
Do you think you can merge this PR now - or there are more things to finish this off?

[Krastanov]

I made a small edit to avoid giving SEO weight to the outdated documentation. There is one last piece that I noticed when I was reading through: the line that you commented out, what is it for, why is it commented out?

[Krastanov]

this line?

[asmirnov69]

yes, in old version this line was ok, but ConcurrentSim.jl was failing. When I moved this line right after yield statement (line 103 in events.md) everything works fine. So you can remove this commented line if there are no reason to investigate further

[Krastanov]

I think this actually introduced a bug: instead of all 3 pupils waiting on the same event, in the second round they all wait on different events, only one of which gets unlocked. I think I fixed that, although I would appreciate if you double check.

I also added a warning above the example to make sure that users know this is a low-level functionality they should not need. This is more easily done with Resource and Store.

[asmirnov69]

yes, i agree, my change was the bug. I forgot at that moment we have more pupil processes so resetting of event after yield does not look right. that also means i need to take a closer look on how succeed() implemented - it seems that you resetting event while it could still block some other yields. Or succeed() have something to guarantee that all pending yields will be processed before succeed() returns?

Looks like I also need to learn how to use Resource and Store.

General question: do you know any written text on web (or just the book/paper) where all this discrete simulation primitives and interactions are described in coherent fashion?

[Krastanov]

Ben wrote this library to mimic SimPy. That could be a useful reference.

[Krastanov]

Tests pass. If you confirm that my fixes did not introduce other bugs, I will merge.

Thanks for taking care of this translation!

[Krastanov]

Thank you for your contribution! It is very much appreciated! If you want to try your hand at anything else in the library, I would be happy to help.