Closed MilanKlausz closed 6 months ago
For 1): Yes, you can add the word "particle" before "generator" to make it more clear (but keep in the "modules").
I am working on some of these now.
Ok, I just pushed fixes for item 1 to item 6.
For your comments on 13), I guess it might be more relevant to mention somewhere that you can simply run:
sb_g4osg_viewgriff -h
Usage:
sb_g4osg_viewgriff [options] <Griff-file>
Display the simulated data (tracks) in the provided Griff file.
Options:
-g, --loadgeo: Construct and display the geometry according to the meta-data
of the Griff file. WARNING: If the code changed since the griff
file was produced, the displayed geometry might different from
the one used while producing the Griff file!!!
-e EVENTS: Only visualise certain events from the file, by specifying event
indicies (i.e. positions in file: 0 is the first event, 99 is the
100th event). The numbers can be provided either directly on the
command line, or by specifying names of text files containing the
numbers. Use commas to separate multiple entries.
(I just checked, the -g option still seems to work!)
I did some of 13 by adding the docs about sb_g4osg_viewgriff, but I think I will leave it there for now.
For 7.... I reread the sections in question again, I am not sure I fully get your point. Is it still valid in the current form of the webpage (which you can't see until it says v0.0.13 on the webpage in ~30mins or so)?
I just pushed v0.0.13 which includes all of the fixes above, apart from the issue about item 7 mentioned in my last comment. I will keep this issue open pending feedback from @MilanKlausz , but will downgrade the severity to not be "high priority" any more, since I don't think it is a blocker.
All issues addressed, and I am satisfied with all the changes!
[x] 1) intro: “Most importantly, dgcode includes functionality for easy seting up and launching of Geant4 simulation jobs based on configurable geometry and generator modules(...)”
The same for “Generators are likewise (...)“, in Features
[x] 2) newsimproject: “_An analysis program which can be invoked via the command sb_tricorderana and which is used to run on the Griff files and which will result in a SimpleHists file being created with analysis histograms.”
The ’and which will results in a (...)” part confusing refers to the ’run on Griff files', while the first ‘and which’ refers to the ana file itself.
“being created with analysis histograms” does this mean “consisting of analysis histograms”?
[x] 3) griff: Extend the “How to enable Griff output” section with info about the multiple griff files created when using -jN (mystuff.0.griff to mystuff.[N-1].griff). (The tip in Analysing Griff files / Basic approach assumes this knowledge later on “Multiple input files can be chained together by specifying them with a wildcard, e.g. mysim.*.griff” ). Maybe put it after “Which changes the default to be to write Griff output in REDUCED mode to a file named mystuff.griff.”
[x] 4) scans: In the example, sample_radius_mm is changed, but in the text it says “The first plot will vary a detector thickness” (it is a bit confusing for me, even if the sample is the detector in this context(?))
[x] 5) simplehists: The ‘Example: Python analysis of histograms’ script at the bottom of the page is supposed to be a "small example of how one can get data out in formats ready to input to the various pylab plotting routines"; which is probably true, but I don't see the need for the “import pylab as pl” line.
[x] 6) mcpl: “_The sb_mcplextrafilterview command can be used to produce a new MCPL file from an existing one, containing only a subset of the original particles. (...)“.
The filterview script is introduced instead of _sb_mcplextrafilterfile, that is actually used in the following examples.
This is the only mentioning of the filterview script/command, so I guess a paragraph dedicated to it is missing.
[x] 7) In newsimproject, it is stated that “For the sake of the following discussions, we will assume you are creating a completely fresh working directory in which you wish to work (this might of course not be the case).” but a little bit later the user is reassured that “Of course, if you are working with code in a shared repository (e.g. at GitHub), nothing has been added to the remote repository at this point. (...)” It is a bit controversial for me. I think this should be restructured a bit to separate the part of creating a bundle (unless working in an existing one), and creating a new package. Restructure part between: “For the sake of the following discussions” AND “Rather, you are now expected to modify the files as appropriate for your actual project before finally git add’ing and committing and pushing them to the central repository server.”
[x] 8 ) particlegenerators: “The is used to select which histogram from the file to use, and the optional part can be specified either as a name or a number. ” For the ‘number’ option, you have to know the default Geant4 unit and provide the conversion factor? (Should we point at some sort of default Geant4 units list?)
[x] 9) particlegenerators: “Note that in addition to the actual bin-contents of the histogram, the sampler also considers the min/max statistics as well as any underflow/overflow content.” How does it consider the underflow/overflow content?
[x] 10) sbparticlegen.rst “As important for a Geant4 simulation as geometry, materials and physics list, is the initial set of particles to be “let loose” in the simulation each event” This might be only confusing for my ear, as I’m not used to the Geant4 meaning of an “event”, can we maybe put in an extra word somewhere? (it is probalbly not important)
The followings are suggested additions:
[x] 11) install: Maybe add a sentence after “_conda env create -f condadgcode.yml”, that depending on the internet connection, it might take several minutes and the resulting conda environment will take up ~3.8G(?) disk space.
[x] 12) newsimproject: simplebuild.cfg file is first mentioned here as “As written, new packages and files have been created for you in your local directory (defaulting to the directory of your master simplebuild.cfg file)“. We might want to link to simplebuild documentation (sbdotcfg or automatic-creation) a little bit earlier when asking the user to invoke
sb --init dgcode
, mentioning that this command will create the (master) simplebuild.cfg file (just to avoid confusion).[x] 13) particlegenerators: At the end of “Generating from Griff files” (after the part about “(...)Griff file was created with a certain filter to select the particles (...)“) Maybe I’d add that together with the visualisation tool (--aim / --data), it can be a very powerful tool for debugging / understanding unexpected events, e.g. by finding unexpected particle paths.