storm icon indicating copy to clipboard operation
storm copied to clipboard

`STORM_PRINT` vs `STORM_LOG_INFO` outside CLI

Open sjunges opened this issue 2 years ago • 7 comments

As posted by @volkm in https://github.com/moves-rwth/storm/issues/271#issuecomment-1202272933_

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

sjunges avatar Aug 02 '22 16:08 sjunges

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?

tquatmann avatar Aug 18 '22 07:08 tquatmann

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.

volkm avatar Aug 29 '22 07:08 volkm

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

sjunges avatar Aug 29 '22 09:08 sjunges

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

volkm avatar Aug 29 '22 09:08 volkm

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

sjunges avatar Sep 13 '22 17:09 sjunges

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.

volkm avatar Sep 14 '22 07:09 volkm

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.

tquatmann avatar Sep 14 '22 08:09 tquatmann