Skip to content
This repository has been archived by the owner on May 6, 2024. It is now read-only.

106 see if kaon properties can be configured at runtime patch #110

Conversation

EinarElen
Copy link
Contributor

This patches a bug introduced in #108 where the ordering of the kaon branching ratios was incorrect

Copy link
Member

@tomeichlersmith tomeichlersmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the work on chasing down that bug 🥇

@tomeichlersmith tomeichlersmith merged commit 79ccb4c into trunk Nov 27, 2023
1 check passed
auto table{kaon->GetDecayTable()};
if (!table) {
EXCEPTION_RAISE("KaonPhysics", "Unable to get the decay table from " +
kaon->GetParticleName());
}
if (verbosity > 1) {
std::cout << "Decay details after setting branching ratios and lifetimes"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and other places, would it be possible to use the logger instead of standard printouts? I see a use case where we use these modifications routinely and would only want to see details at debug or info level.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, although I'm not familiar with how to do that with things in LDMX-sw that aren't processors

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically ldmx_log is a macro that expects the logger to be a variable named theLog_

I also wrote another macro enableLogging that you could put in the class declaration which defines theLog_ as a member variable making it accessible to all member functions.

https://ldmx-software.github.io/Logging.html#more-detail-logging-outside-processors

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me neither :) hence the hesitation. To be clear I think a combination of the local verbosity and the log level is the best way to do it in that case, so this suggestion is just about where the message is directed.

Copy link
Contributor Author

@EinarElen EinarElen Nov 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, was straight-forward to do although the enableLogging macro didn't play nicely by default with the namespace I was in so had to do

  mutable framework::logging::logger theLog_{
      framework::logging::makeLogger("KaonPhysics")};

directly. Not the end of the world I guess

You run into something like

/Users/einarelen/ldmx/ldmx-sw/Framework/include/Framework/Logger.h:137:11: error: 'logging' does not name a type; did you mean 'long'?
  137 |   mutable logging::logger theLog_{logging::makeLogger(name)};
      |           ^~~~~~~
/Users/einarelen/ldmx/ldmx-sw/Framework/include/Framework/Logger.h:137:11: note: in definition of macro 'enableLogging'
  137 |   mutable logging::logger theLog_{logging::makeLogger(name)};
      |           ^~~~~~~

*
* and
*
* K^0_S -> \pi^+ + \pi^-
* K^0_S -> \pi^0 + \pi^0
*
* @note: The order in the the decay table is sorted by the branching ratios
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this should be "must be" rather than "is", if I understood the bug correctly? If I'm right, I think the stronger language will be more helpful for the future developer reading the comment

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and same on L52

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

Successfully merging this pull request may close these issues.

3 participants