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

More messenger commands for physics lists #254

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

tdixon97
Copy link
Collaborator

@tdixon97 tdixon97 commented Feb 6, 2025

I need to test it

include/RMGHardware.hh Outdated Show resolved Hide resolved
include/RMGHardware.hh Show resolved Hide resolved
include/RMGPhysics.hh Outdated Show resolved Hide resolved
include/RMGPhysics.hh Outdated Show resolved Hide resolved
include/RMGHardwareMessenger.hh Outdated Show resolved Hide resolved
src/RMGHardwareMessenger.cc Outdated Show resolved Hide resolved
@tdixon97
Copy link
Collaborator Author

tdixon97 commented Feb 7, 2025

Any idea why my new macro command isn't picked up by rmg-doc-dump? I did run it

@ManuelHu
Copy link
Collaborator

ManuelHu commented Feb 7, 2025

That would be a very curious case, as the CI also just runs make remage-doc-dump... (just running remage-doc-dump without make is not enough)

Comment on lines 81 to 82
step_limit->SetGuidance("The maximum step limit");
step_limit->SetDefaultValue(1 * CLHEP::mm);
Copy link
Collaborator

Choose a reason for hiding this comment

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

there is also a way to have parameters with units, that would be nicer. Or, at least mention the unit this is in. Currently this just renders as "default value - 1" without any unit information in the docs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am working on it, but I had to change this into two macro commands

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now this is done

Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't have to split it in two commands... I have some example code how to use G4UIcmdWithADoubleAndUnit with extra parameters:

diff --git a/include/RMGHardwareMessenger.hh b/include/RMGHardwareMessenger.hh
index 8ee2873..bcceed3 100644
--- a/include/RMGHardwareMessenger.hh
+++ b/include/RMGHardwareMessenger.hh
@@ -17,6 +17,7 @@
 #define _RMG_HARDWARE_MESSENGER_HH_
 
 #include "G4UImessenger.hh"
+#include "G4UIcmdWithADoubleAndUnit.hh"
 
 class RMGHardware;
 class RMGHardwareMessenger : public G4UImessenger {
@@ -31,7 +32,7 @@ class RMGHardwareMessenger : public G4UImessenger {
   private:
 
     RMGHardware* fHardware;
-    G4UIcommand* fRegisterCmd;
+    G4UIcmdWithADoubleAndUnit* fRegisterCmd;
 
     void RegisterDetectorCmd(const std::string& parameters);
 };
diff --git a/src/RMGHardwareMessenger.cc b/src/RMGHardwareMessenger.cc
index aa200af..1a4bdd8 100644
--- a/src/RMGHardwareMessenger.cc
+++ b/src/RMGHardwareMessenger.cc
@@ -17,14 +17,19 @@
 
 #include "G4Tokenizer.hh"
 #include "G4UIcommand.hh"
+#include "G4UIcmdWithADoubleAndUnit.hh"
 
 #include "RMGHardware.hh"
 #include "RMGTools.hh"
 
 RMGHardwareMessenger::RMGHardwareMessenger(RMGHardware* hw) : fHardware(hw) {
-  fRegisterCmd = new G4UIcommand("/RMG/Geometry/RegisterDetector", this);
+  fRegisterCmd = new G4UIcmdWithADoubleAndUnit("/RMG/Geometry/RegisterDetector", this);
   fRegisterCmd->SetGuidance("register a sensitive detector");
 
+  fRegisterCmd->SetParameterName("test", false);
+  fRegisterCmd->SetDefaultValue(5);
+  fRegisterCmd->SetUnitCategory("Length");
+
   auto p_type = new G4UIparameter("type", 's', false);
   p_type->SetParameterCandidates(RMGTools::GetCandidates<RMGDetectorType>().c_str());
   p_type->SetGuidance("Detector type");
@@ -59,9 +64,15 @@ void RMGHardwareMessenger::SetNewValue(G4UIcommand* command, G4String newValues)
 }
 
 void RMGHardwareMessenger::RegisterDetectorCmd(const std::string& parameters) {
+  RMGLog::Out(RMGLog::error, "params ", parameters);
   G4Tokenizer next(parameters);
 
+  auto x = next();
+  auto y = next();
+  auto num = G4UIcmdWithADoubleAndUnit::GetNewDoubleValue((x + " " + y).c_str());
+
   auto type_str = next();
+  RMGLog::Out(RMGLog::fatal, " num ", num);
   auto type = RMGTools::ToEnum<RMGDetectorType>(std::string(type_str), "detector type");
   auto pv_name = next();
   const int uid = std::stoi(next());

@tdixon97
Copy link
Collaborator Author

tdixon97 commented Feb 7, 2025

The step limiting seems to be working, we could have a nicer interface (eg. specifying with regex) but I am not sure since I doubt its something we will use.

Here is an example with 10 um step limits. Of course now the simulation is extremely slow

image

@gipert
Copy link
Member

gipert commented Feb 7, 2025

Seems good to me.

@tdixon97
Copy link
Collaborator Author

tdixon97 commented Feb 7, 2025

I would wait until its more tested to merge.

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

Successfully merging this pull request may close these issues.

3 participants