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

STORM_PRINT vs STORM_LOG_INFO outside CLI #272

Open
sjunges opened this issue Aug 2, 2022 · 7 comments
Open

STORM_PRINT vs STORM_LOG_INFO outside CLI #272

sjunges opened this issue Aug 2, 2022 · 7 comments

Comments

@sjunges
Copy link
Contributor

sjunges commented Aug 2, 2022

As posted by @volkm in #271 (comment)_

we have quite some STORM_PRINTs which do appear when calling python bindings. In most cases, uses STORM_LOG_INFO is preferable.

@tquatmann
Copy link
Member

tquatmann commented Aug 18, 2022

This is mildly related to #276 and #277

The more I think about it: is there any reasonable use of STORM_PRINT outside of CLI code?

@volkm
Copy link
Contributor

volkm commented Aug 29, 2022

I agree with Tim that we should only use STORM_PRINT in the CLI code. That way, other libraries building upon Storm (such as stormpy) do not have any output from Storm by default. If any intermediate output is necessary, one could always fall-back to STORM_LOG_INFO instead.

@sjunges
Copy link
Contributor Author

sjunges commented Aug 29, 2022

We can enforce this by moving the macro for STORM_PRINT to cli-utilities?

@volkm
Copy link
Contributor

volkm commented Aug 29, 2022

Good idea. That means moving them from src/storm/utility/macros.h to somewhere in src/storm-cli-utilities.

@sjunges
Copy link
Contributor Author

sjunges commented Sep 13, 2022

We have quite some of these guarded by storm::settings::getModule<storm::settings::modules::CoreSettings>().isShowStatisticsSet(). What should we do about them?

@volkm
Copy link
Contributor

volkm commented Sep 14, 2022

Maybe either use STORM_LOG_INFO here with the existing guard, or introduce another logging level STORM_LOG_STATISTICS? Depending on how much overhead we want to create, we could also introduce a statistics object which can be filled by the algorithms and all relevant information can be accessed later on.

@tquatmann
Copy link
Member

My vote is for STORM_LOG_STATISTICS (and STORM_LOG_PROGRESS for similar purposes). I think it's the most flexible option. If we want, we could just use it as an alias for STORM_LOG_INFO for now, and implement something smarter later.

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

No branches or pull requests

3 participants