reduce default printing #651

rlcee · 2021-11-05T01:17:28Z

No description provided.

FNALbuild · 2021-11-05T01:17:58Z

Hi @rlcee,
You have proposed changes to files in these packages:

fcl
Mu2eG4
EventMixing

which require these tests: build.

@Mu2e/write, @Mu2e/fnalbuild-users have access to CI actions on main.

The following users requested to be notified about changes to these packages:
@resnegfk

⌛ The following tests have been triggered for b2773e3: build (Build queue has 1 jobs)

About FNALbuild. Code review on Mu2e/Offline.

FNALbuild · 2021-11-05T01:43:50Z

☀️ The tests passed at b2773e3.

Test	Result	Details
merge	✅	Merged `b2773e3` at `40921f4`
build (prof)	✅	Log file. Build time: 13 min 17 sec
ceSimReco	✅	Log file.
g4test_03MT	✅	Log file.
transportOnly	✅	Log file.
POT	✅	Log file.
g4study	✅	Log file.
cosmicSimReco	✅	Log file.
cosmicOffSpill	✅	Log file.
ceSteps	✅	Log file.
ceDigi	✅	Log file.
muDauSteps	✅	Log file.
ceMix	✅	Log file.
rootOverlaps	✅	Log file.
g4surfaceCheck	✅	Log file.
FIXME, TODO	✅	TODO (0) FIXME (0) in 2 files
clang-tidy	〰️	0 errors 0 warnings

N.B. These results were obtained from a build of this Pull Request at b2773e3 after being merged into the base branch at 40921f4.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

gaponenko · 2021-11-05T15:55:36Z

fcl/standardMessageDestinations.fcl

@@ -138,6 +138,8 @@ mf_coutInfo:
   Configuration : { limit : 0 }
   HITS : { limit : 0 }
   COSMIC_STEPPOINTS : { limit : 0 }
+   Summary : { limit : 0 }
+   INFO : { limit : 0 }
 } # end categories


Does limit:0 disable the printing?

Does limit:0 disable the printing?

Yes, the number is how many prints of this message are allowed,
so zero means no printing

I believe that if you ask for the message facility end of job summary report you get the number of messages in each category, but only for categories with > 0 messages. So there is value in setting a limit of 0 as a default.

Ray: do I have that right? Do we have that summary on by default?

Hi Ray,

Yes, the number is how many prints of this message are allowed, so zero means no printing

What is not clear to me is the role of all those INFO printouts to message facility in the code.

I agree that we want to minimize routine "everything is OK" printouts in production jobs, but there is always a tension between production and code development. If a new developer starts working on an Offline module, ideally things would be setup for all the info printouts to show up in their tests, I think. But the same printouts would be off for production running.

When I first started working on Offline, I realized that not all my prints to message_facility showed up (I do not remember the details). I guessed there were some obscure settings somewhere to make them work, but that complexity immediately disqualified the use of message facility for code development/debugging in my mind. I just started printing all the stuff I needed to see to std::cout. I am not saying that this is the right thing to do, but this is how it worked in practice.

I think the "out-of-the-box" setting of Offline should be for production running. If we can have a simple instruction for code developers, "after git clone and muse setup do this to enable INFO printouts in all your interactive jobs", then having all those message facility prints in our C++ code makes sense. Otherwise they are basically dead code, not used in production, not used for code development. May be just for experts to localize a problem without re-compiling the code.

I think addressing this question is outside of the scope of the current PR, which I am approving, but we should keep thinking about this.
Andrei

I agree that this is out of scope. I have relaunched the tests will merge. I will create an issue to follow up.

gaponenko · 2021-11-05T17:37:27Z

fcl/standardMessageDestinations.fcl

@@ -138,6 +138,8 @@ mf_coutInfo:
   Configuration : { limit : 0 }
   HITS : { limit : 0 }
   COSMIC_STEPPOINTS : { limit : 0 }
+   Summary : { limit : 0 }
+   INFO : { limit : 0 }
 } # end categories


Hi Ray,

Yes, the number is how many prints of this message are allowed, so zero means no printing

What is not clear to me is the role of all those INFO printouts to message facility in the code.

I agree that we want to minimize routine "everything is OK" printouts in production jobs, but there is always a tension between production and code development. If a new developer starts working on an Offline module, ideally things would be setup for all the info printouts to show up in their tests, I think. But the same printouts would be off for production running.

When I first started working on Offline, I realized that not all my prints to message_facility showed up (I do not remember the details). I guessed there were some obscure settings somewhere to make them work, but that complexity immediately disqualified the use of message facility for code development/debugging in my mind. I just started printing all the stuff I needed to see to std::cout. I am not saying that this is the right thing to do, but this is how it worked in practice.

I think the "out-of-the-box" setting of Offline should be for production running. If we can have a simple instruction for code developers, "after git clone and muse setup do this to enable INFO printouts in all your interactive jobs", then having all those message facility prints in our C++ code makes sense. Otherwise they are basically dead code, not used in production, not used for code development. May be just for experts to localize a problem without re-compiling the code.

I think addressing this question is outside of the scope of the current PR, which I am approving, but we should keep thinking about this.
Andrei

rlcee · 2021-11-05T18:21:39Z

Andrei, you raise good points. In my talk, I offered to provide other settings. Right now all our fcl defaults to mf_interactive. The original plan, which you can see in the files is a different set of settings for production and for debug etc. If you set
services.message : @Local::mf_debugging
you will get the message back, but in error.log
There are already mf_warning, presumably only warning or higher, etc
I'm happy to provide any preference with any name, I just need to know what those suggestions are. I'll make sure there is one which is unsuppressed and writes to cout.

Another point I am concerned about, which everyone should be aware of, is that you can only suppress individual error messages by their short name, "Summary" for the filters. These strings really should really be unique to the world, like "GenFilterSummary". This way, we can control each individually, for example if there are occasional corrupt data, we might want to print the first 3 occurrences of corruption X, but all occurrences of corruption Y, if you see what I mean. You can't suppress them by code source. You can suppress all of the same level - info, warning, error.

FNALbuild · 2021-11-05T18:22:07Z

📝 The HEAD of main has changed to 09c73fd. Tests are now out of date.

kutschke · 2021-11-05T18:23:20Z

@FNALbuild run build test

FNALbuild · 2021-11-05T18:23:47Z

⌛ The following tests have been triggered for b2773e3: build (Build queue has 1 jobs)

kutschke · 2021-11-05T18:45:10Z

I created two new issues to followup on the two topics that came up during this discussion.

FNALbuild · 2021-11-05T18:49:17Z

☀️ The tests passed at b2773e3.

Test	Result	Details
merge	✅	Merged `b2773e3` at `09c73fd`
build (prof)	✅	Log file. Build time: 13 min 01 sec
ceSimReco	✅	Log file.
g4test_03MT	✅	Log file.
transportOnly	✅	Log file.
POT	✅	Log file.
g4study	✅	Log file.
cosmicSimReco	✅	Log file.
cosmicOffSpill	✅	Log file.
ceSteps	✅	Log file.
ceDigi	✅	Log file.
muDauSteps	✅	Log file.
ceMix	✅	Log file.
rootOverlaps	✅	Log file.
g4surfaceCheck	✅	Log file.
FIXME, TODO	✅	TODO (0) FIXME (0) in 2 files
clang-tidy	〰️	0 errors 0 warnings

N.B. These results were obtained from a build of this Pull Request at b2773e3 after being merged into the base branch at 09c73fd.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

reduce default printing

b2773e3

FNALbuild added build pending build running and removed build pending labels Nov 5, 2021

FNALbuild added build finished and removed build running labels Nov 5, 2021

brownd1978 assigned kutschke Nov 5, 2021

brownd1978 requested a review from gaponenko November 5, 2021 14:17

gaponenko reviewed Nov 5, 2021

View reviewed changes

gaponenko approved these changes Nov 5, 2021

View reviewed changes

FNALbuild added build pending and removed build finished labels Nov 5, 2021

FNALbuild added build running and removed build pending labels Nov 5, 2021

kutschke mentioned this pull request Nov 5, 2021

Refactor Message Categories thinking ahead to Operations #654

Closed

This was linked to issues Nov 5, 2021

Management of Message Verbosity #653

Closed

Refactor Message Categories thinking ahead to Operations #654

Closed

kutschke approved these changes Nov 5, 2021

View reviewed changes

FNALbuild added build finished and removed build running labels Nov 5, 2021

kutschke merged commit b5c4c45 into Mu2e:main Nov 5, 2021

rlcee mentioned this pull request Nov 20, 2021

filter and CRV summary log text #640

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reduce default printing #651

reduce default printing #651

rlcee commented Nov 5, 2021

FNALbuild commented Nov 5, 2021

FNALbuild commented Nov 5, 2021

gaponenko Nov 5, 2021

rlcee Nov 5, 2021

kutschke Nov 5, 2021

gaponenko Nov 5, 2021 •

edited

Loading

kutschke Nov 5, 2021 •

edited

Loading

gaponenko Nov 5, 2021 •

edited

Loading

rlcee commented Nov 5, 2021

FNALbuild commented Nov 5, 2021

kutschke commented Nov 5, 2021

FNALbuild commented Nov 5, 2021

kutschke commented Nov 5, 2021

FNALbuild commented Nov 5, 2021

reduce default printing #651

reduce default printing #651

Conversation

rlcee commented Nov 5, 2021

FNALbuild commented Nov 5, 2021

FNALbuild commented Nov 5, 2021

gaponenko Nov 5, 2021

Choose a reason for hiding this comment

rlcee Nov 5, 2021

Choose a reason for hiding this comment

kutschke Nov 5, 2021

Choose a reason for hiding this comment

gaponenko Nov 5, 2021 • edited Loading

Choose a reason for hiding this comment

kutschke Nov 5, 2021 • edited Loading

Choose a reason for hiding this comment

gaponenko Nov 5, 2021 • edited Loading

Choose a reason for hiding this comment

rlcee commented Nov 5, 2021

FNALbuild commented Nov 5, 2021

kutschke commented Nov 5, 2021

FNALbuild commented Nov 5, 2021

kutschke commented Nov 5, 2021

FNALbuild commented Nov 5, 2021

gaponenko Nov 5, 2021 •

edited

Loading

kutschke Nov 5, 2021 •

edited

Loading

gaponenko Nov 5, 2021 •

edited

Loading