search

Mu2e / Production

Scripts and fcl for collaboration production procedures
Apache License 2.0
0 stars 33 forks source link

Cosmic S2 - second batch of changes. #313

Closed oksuzian closed 4 months ago

FNALbuild commented 4 months ago

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

which require these tests: build.

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

:hourglass: The following tests have been triggered for 654bef5603b90fd0c86480018e84ea30efc1145a: build (Build queue has 1 jobs)

About FNALbuild. Code review on Mu2e/Offline.

FNALbuild commented 4 months ago

:sunny: The build tests passed at 654bef5603b90fd0c86480018e84ea30efc1145a.

Test Result Details
test with :white_check_mark: Command did not list any other PRs to include
merge :white_check_mark: Merged 654bef5603b90fd0c86480018e84ea30efc1145a at dea81d6fcbc9556c56bb637da94feab725c52afb
build (prof) :white_check_mark: Log file. Build time: 11 min 37 sec
ceSimReco :white_check_mark: Log file.
g4test_03MT :white_check_mark: Log file.
transportOnly :white_check_mark: Log file.
POT :white_check_mark: Log file.
g4study :white_check_mark: Log file.
cosmicSimReco :white_check_mark: Log file.
cosmicOffSpill :white_check_mark: Log file.
ceSteps :white_check_mark: Log file.
ceDigi :white_check_mark: Log file.
muDauSteps :white_check_mark: Log file.
ceMix :white_check_mark: Log file.
rootOverlaps :white_check_mark: Log file.
g4surfaceCheck :white_check_mark: Log file.
FIXME, TODO :white_check_mark: TODO (0) FIXME (0) in 0 files
clang-tidy :white_check_mark: 0 errors 0 warnings

N.B. These results were obtained from a build of this Pull Request at 654bef5603b90fd0c86480018e84ea30efc1145a after being merged into the base branch at dea81d6fcbc9556c56bb637da94feab725c52afb.

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.

brownd1978 commented 4 months ago

There are also 'low' in the POMS scripts. I think it would be easier to understand if a single capitalization were used everywhere.

On Mon, Apr 29, 2024 at 1:22 PM oksuzian @.***> wrote:

@.**** commented on this pull request.

In Scripts/gen_CosmicStage2.sh https://github.com/Mu2e/Production/pull/313#discussion_r1583701394:

@@ -82,10 +87,16 @@ if [[ ${NJOBS} == 0 || ${NEVTS} == 0 ]]; then exit_abnormal fi

+if [[ ${LOW} != "Low" && ${LOW} != "All" ]]; then

The original code had "--low Low" option. I've added "--low All". Do you want to change it to "--Low Low"?

— Reply to this email directly, view it on GitHub https://github.com/Mu2e/Production/pull/313#discussion_r1583701394, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH573CHUK75NB6TUJCZ33Y72TZRAVCNFSM6AAAAABG6XWHH2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMRZGU2DEOBWGY . You are receiving this because you are on a team that was mentioned.Message ID: @.***>

-- David Brown @.*** Office Phone (510) 486-7261 Lawrence Berkeley National Lab M/S 50R5008 (50-6026C) Berkeley, CA 94720

brownd1978 commented 4 months ago

On Mon, Apr 29, 2024 at 1:26 PM oksuzian @.***> wrote:

@.**** commented on this pull request.

In Scripts/gen_CosmicStage2.sh https://github.com/Mu2e/Production/pull/313#discussion_r1583706481:

@@ -31,6 +32,7 @@ usage() { [ --low Resample 'Low' S1 output ]

I don't think All is the default. --low is the required option.

Oh, I see: --low "All" is a valid command. That's unintuitive to me. Either the parameter name should change (--S1Type or something) or --low should an optional (Boolean) with 'All' as default.

— Reply to this email directly, view it on GitHub https://github.com/Mu2e/Production/pull/313#discussion_r1583706481, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH574ACYVIQNK3E35AZLTY72UH5AVCNFSM6AAAAABG6XWHH2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMRZGU2TQNJVGM . You are receiving this because you are on a team that was mentioned.Message ID: @.***>

-- David Brown @.*** Office Phone (510) 486-7261 Lawrence Berkeley National Lab M/S 50R5008 (50-6026C) Berkeley, CA 94720

oksuzian commented 4 months ago

On Mon, Apr 29, 2024 at 1:26 PM oksuzian @.> wrote: @*.*** commented on this pull request. ------------------------------ In Scripts/gen_CosmicStage2.sh <#313 (comment)>: > @@ -31,6 +32,7 @@ usage() { [ --low Resample 'Low' S1 output ] > I don't think All is the default. > --low is the required option. Oh, I see: --low "All" is a valid command. That's unintuitive to me. Either the parameter name should change (--S1Type or something) or --low should an optional (Boolean) with 'All' as default. … — Reply to this email directly, view it on GitHub <#313 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH574ACYVIQNK3E35AZLTY72UH5AVCNFSM6AAAAABG6XWHH2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMRZGU2TQNJVGM . You are receiving this because you are on a team that was mentioned.Message ID: @.> -- David Brown @.*** Office Phone (510) 486-7261 Lawrence Berkeley National Lab M/S 50R5008 (50-6026C) Berkeley, CA 94720

--low is the required option. The argument is also used in POMS configs. I can't have the default, undefined option. I'll do --S1Type Should I change the following to --S1Gen? [ --S1 Cosmic S1 name (CRY, CORSIKA, ...) ]

brownd1978 commented 4 months ago

On Mon, Apr 29, 2024 at 1:43 PM oksuzian @.***> wrote:

On Mon, Apr 29, 2024 at 1:26 PM oksuzian @.*> wrote: @.* commented on this pull request. ------------------------------ In Scripts/gen_CosmicStage2.sh <#313 (comment) https://github.com/Mu2e/Production/pull/313#discussion_r1583706481>: > @@ -31,6 +32,7 @@ usage() { [ --low Resample 'Low' S1 output ] > I don't think All is the default. > --low is the required option. Oh, I see: --low "All" is a valid command. That's unintuitive to me. Either the parameter name should change (--S1Type or something) or --low should an optional (Boolean) with 'All' as default. … <#m-8912663168129233673> — Reply to this email directly, view it on GitHub <#313 (comment) https://github.com/Mu2e/Production/pull/313#discussion_r1583706481>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH574ACYVIQNK3E35AZLTY72UH5AVCNFSM6AAAAABG6XWHH2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMRZGU2TQNJVGM . You are receiving this because you are on a team that was mentioned.Message ID: @. > -- David Brown @.* Office Phone (510) 486-7261 Lawrence Berkeley National Lab M/S 50R5008 (50-6026C) Berkeley, CA 94720

--low is the required option. The argument is also used in POMS configs. I can't have the default, undefined option. I'll do --S1Type Should I change the following to --S1Gen? [ --S1 Cosmic S1 name (CRY, CORSIKA, ...) ]

Sure if you prefer.

— Reply to this email directly, view it on GitHub https://github.com/Mu2e/Production/pull/313#issuecomment-2083628424, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH574ANJZCZFEQWFDX2P3Y72WHRAVCNFSM6AAAAABG6XWHH2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBTGYZDQNBSGQ . You are receiving this because you are on a team that was mentioned.Message ID: @.***>

-- David Brown @.*** Office Phone (510) 486-7261 Lawrence Berkeley National Lab M/S 50R5008 (50-6026C) Berkeley, CA 94720