Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: Handle partially done flows #6934

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

graux-pierre
Copy link

Description

When a block that produces outputs is connected to several blocks, either it has several outputs or one of its outputs is connected to several blocks, if one of the connected blocks is done (were_done in the scheduler) then the block is also considered done. This is problematic since, in order to keep the flow live, we need to keep a useless block (the one that should be done) on a working state, which is both counter intuitive and CPU-consuming.

This issue is solve differently for the multiple outputs case or the output connected to multiple readers case:

  • multiple outputs: when the block executor computes how many space is left in output buffers of a block (min_available_space function), outputs that are marked as done are not taken into account.

  • multiple readers: when a buffer_reader is set to done, instead of setting the whole linked buffer to done, the buffer reader notifies the buffer so the buffer can keep track which reader it should take into account when computing space.

In order to keep backward compatibility, I have added an attribute to the blocks (basic_block class) that controls how the scheduler behaves and which is set by default to the former behavior. This attribute has been added to the advanced tab of the GRC parameters window, next to other buffer options.

Related Issue

No related issues found.

Which blocks/areas does this affect?

runtime: block_executor, basic_block and buffers.
grc: parameter interface (advanced tab)

Testing [Done]

Tests (grc files) for the two behaviors described above: tests_partial_flow.zip

Note that, at least for me, cmake should be run again to avoid compilation errors related to basic_block placeholders for docstrings (gnuradio-runtime/python/gnuradio/gr/bindings/docstrings/basic_block_pydoc_template.h).

Checklist

When a block has several outputs or when a block has an output that is
connected to several blocks, if one of the reading blocks is done then
the writing block is considered as done. This commit allows to specify
when a writing block should only be stopped when all its readers are
done. The former behavior is kept as the default one.

Signed-off-by: GRAUX Pierre <pierre.graux@univ-lille.fr>
Add the possibility to specify when a block doesn't need all its outputs
to work and should only be stopped when all its readers are done.

Signed-off-by: GRAUX Pierre <pierre.graux@univ-lille.fr>
@willcode
Copy link
Member

@graux-pierre Could you come up with an example flowgraph where the current behavior is causing a problem? We admit that flowgraph shutdown is not clean. We need to be very careful making changes to the scheduler because it could affect different workloads in different ways.

In general, I'd rather make improvements to the scheduler that work in all cases, and not have a block-by-block difference in behavior based on a user parameter. That gets confusing really fast.

@graux-pierre
Copy link
Author

If you want a minimal non-working example you can use the two grc files provided in the "Testing" section of the PR message: here

I can't provide you the actual flowgraph that I'm using since it isn't cleaned and the work still unpublished. This flowgraph is composed of a source stream that is divided into two sets: the first N values are used to train a machine learning model and the remaining ones are used to test this model. When the training is over, the corresponding block is done and so the division block is stopped and the testing is never done.

                                        ---------- 
                               ------> | Training |--------
 --------        ----------    |        ----------        |
| Source |----->| Division |----                          |
 --------       |          |----                          v
                 ----------    |                      --------- 
                               --------------------->| Testing |
                                                      --------- 

I think that the scheduler should never stop block if not all outputs are done. I've only added the block attribute in the case some workflows rely on the former behavior to stop but relying on an internal behavior of the scheduler instead of properly stopping blocks doesn't seem to be a good practice.

Also, I think this feature add the possibility to have optional output streams which I guess (not tested) shouldn't work with the current scheduler state. But this might be redundant with null sink blocks.

@willcode willcode marked this pull request as draft October 31, 2023 14:18
@willcode
Copy link
Member

Set to "draft" so the conversation can continue. I don't think it's likely we'll do exactly this, but there might be something similar and more general.

@graux-pierre
Copy link
Author

This conversation may be linked with the one of the issue #6792.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants