search

Mu2e / Offline

Offline software for the Mu2e experiment
Apache License 2.0
8 stars 79 forks source link

Define nominal database for Offline code testing #1261

Closed brownd1978 closed 2 months ago

brownd1978 commented 2 months ago

This addresses comments in PR 1260

FNALbuild commented 2 months ago

Hi @brownd1978, 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 342b23cab5d9eec42faa8f039ba91d4406360242: build (Build queue is empty)

About FNALbuild. Code review on Mu2e/Offline.

brownd1978 commented 2 months ago

Adding that stanza there will cause every standard job to initialize the database, whether it needs it or not.

On Tue, May 7, 2024 at 1:13 PM Ray Culbertson @.***> wrote:

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

We have a nominal here https://github.com/Mu2e/Offline/blob/main/DbService/fcl/prolog.fcl here https://github.com/Mu2e/Offline/blob/7eeafee28e3390039ea022d841c363d88ad7782e/fcl/standardServices.fcl#L48. Think the thing to do is provide a new stanza in the DbService prolog and then use that where you need to

— Reply to this email directly, view it on GitHub https://github.com/Mu2e/Offline/pull/1261#pullrequestreview-2044089178, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH573P7FBCGF5TAWLU7ATZBEYW7AVCNFSM6AAAAABHLU3R3CVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANBUGA4DSMJXHA . You are receiving this because you were mentioned.Message ID: @.***>

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

FNALbuild commented 2 months ago

:sunny: The build tests passed at 342b23cab5d9eec42faa8f039ba91d4406360242.

Test Result Details
test with :white_check_mark: Command did not list any other PRs to include
merge :white_check_mark: Merged 342b23cab5d9eec42faa8f039ba91d4406360242 at 7eeafee28e3390039ea022d841c363d88ad7782e
build (prof) :white_check_mark: Log file. Build time: 11 min 24 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
whitespace check :white_check_mark: no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 342b23cab5d9eec42faa8f039ba91d4406360242 after being merged into the base branch at 7eeafee28e3390039ea022d841c363d88ad7782e.

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.

rlcee commented 2 months ago

Adding that stanza there will cause every standard job to initialize the database, whether it needs it or not.

Yes, my main point was the stanza should go in DbService, not ConditionsService, then were it is used is up to you, I was just showing where the "empty" is used.

kutschke commented 2 months ago

Based on the discussion to date, does this plan work:

  1. Dave: rename the file to the directory that Ray recommends
  2. Ray: approve
  3. Rob: merge (no need to rerun CI)

And some followup questions:

  1. What about Production/Validation/database.fcl ? Should we change it to #include the new file? This lets us modify it later as needed.
  2. What's the status of Production/JobConfig/reco/misalign_epilog.fcl - it still uses a text file?

All other fcl that references DbService has test or Test in it's name. I will leave those for the authors who are presiumably experts.

kutschke commented 2 months ago

.... and step 4: Ralf update his PR to use this one. then we can approve and merge that one.

brownd1978 commented 2 months ago

On Tue, May 7, 2024 at 1:48 PM Rob Kutschke @.***> wrote:

Based on the discussion to date, does this plan work:

  1. Dave: rename the file to the directory that Ray recommends
  2. Ray: approve
  3. Rob: merge (no need to rerun CI)

And some followup questions:

  1. What about Production/Validation/database.fcl ? Should we change it to #include the new file? This lets us modify it later as needed.

Ostensibly that serves a different purpose (db version for CI and validation). I also don't like having Offline depend on Production (outside of test scripts)

  1. What's the status of Production/JobConfig/reco/misalign_epilog.fcl
    • it still uses a text file?

How is this connected with the topic of Ralf's PR?

All other fcl that references DbService has test or Test in it's name. I will leave those for the authors who are presiumably experts.

— Reply to this email directly, view it on GitHub https://github.com/Mu2e/Offline/pull/1261#issuecomment-2099281769, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH5734EYDLNEFOQN47JWLZBE42NAVCNFSM6AAAAABHLU3R3CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJZGI4DCNZWHE . You are receiving this because you were mentioned.Message ID: @.***>

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

bonventre commented 2 months ago

Assuming this one is more just a version of the database that is functional with this version of offline and Validation/database.fcl is assumed to be up to date latest greatest, I feel like we need some extra warning printout or something if you include this file instead

kutschke commented 2 months ago

And some followup questions: 1. What about Production/Validation/database.fcl ? Should we change it to #include the new file? This lets us modify it later as needed.

Ostensibly that serves a different purpose (db version for CI and validation). I also don't like having Offline depend on Production (outside of test scripts)

My proposal is for Production to include Offline, which is regular.

  1. What's the status of Production/JobConfig/reco/misalign_epilog.fcl - it still uses a text file?

How is this connected with the topic of Ralf's PR?

We are now providing a default. I looked for other places that set DbService parameters by hand to ask if it makes sense to change them too.

kutschke commented 2 months ago

@rlcee Dave has made the change you requested. If you are OK with it, please approve. Thanks

kutschke commented 2 months ago

Running CI to test the new Jenkins machine

@FNALbuild run build test

FNALbuild commented 2 months ago

:hourglass: The following tests have been triggered for 9b3aae627b6b14934de7865faea1a6f8419e803b: build (Build queue is empty)

FNALbuild commented 2 months ago

:sunny: The build tests passed at 9b3aae627b6b14934de7865faea1a6f8419e803b.

Test Result Details
test with :white_check_mark: Command did not list any other PRs to include
merge :white_check_mark: Merged 9b3aae627b6b14934de7865faea1a6f8419e803b at 7eeafee28e3390039ea022d841c363d88ad7782e
build (prof) :white_check_mark: Log file. Build time: 11 min 04 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
whitespace check :white_check_mark: no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 9b3aae627b6b14934de7865faea1a6f8419e803b after being merged into the base branch at 7eeafee28e3390039ea022d841c363d88ad7782e.

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.