Skip to content
New issue

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

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

Already on GitHub? # to your account

Zero breadth warning can be improved #1181

Closed
lhstrh opened this issue May 19, 2022 · 4 comments
Closed

Zero breadth warning can be improved #1181

lhstrh opened this issue May 19, 2022 · 4 comments
Assignees
Labels
enhancement Enhancement of existing feature

Comments

@lhstrh
Copy link
Member

lhstrh commented May 19, 2022

We currently have this warning message:

Reaction graph breadth is computed to be 0. Indicates an error.

Perhaps it makes more sense to report something less cryptic, like No reactions in this program.

@lhstrh lhstrh added the enhancement Enhancement of existing feature label May 19, 2022
@erlingrj
Copy link
Collaborator

erlingrj commented May 19, 2022

Agreed. Should I also add the smarter-default-number-of-workers to the Cpp target?

@lhstrh
Copy link
Member Author

lhstrh commented May 19, 2022

Should I also add the smarter-default-number-of-workers to the Cpp target?

The dependency graph is constructed at runtime in reactor-cpp, so I doubt that if we want this feature for the C++ target we'd want it implemented at compile time. That said, I think that @cmnrd should comment on this :-)

@cmnrd
Copy link
Collaborator

cmnrd commented Jun 20, 2022

I am not sure what exactly 'smarter-default-number-of-workers' refers to, but indeed we shouldn't aim at statically setting the number of workers in C++. Currently, the C++ runtime default is to use as many workers as there are hardware threads on the machine executing the program. If there is evidence that a more complex strategy brings significant benefits, I would be happy to accept a PR that implements this strategy in reactor-cpp.

@cmnrd
Copy link
Collaborator

cmnrd commented Jun 20, 2022

Should we close this? Looks like there is an associated merged PR.

@cmnrd cmnrd closed this as completed Jul 6, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement Enhancement of existing feature
Projects
None yet
Development

No branches or pull requests

3 participants