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

Latest progress on ensembles scripts #328

Merged
merged 27 commits into from
Aug 5, 2024
Merged

Latest progress on ensembles scripts #328

merged 27 commits into from
Aug 5, 2024

Conversation

sophiemiddleton
Copy link
Collaborator

PR contains new scripts to help produce ensembles

@FNALbuild
Copy link
Collaborator

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

  • CampaignConfig
  • Scripts
  • JobConfig

which require these tests: build.

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

⌛ The following tests have been triggered for fd1e2ef: build (Build queue has 1 jobs)

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild
Copy link
Collaborator

‼️ It was not possible to prepare the workspace for this test. This is often caused by merge conflicts - please check and try again.

> git diff --check | grep -i conflict
CampaignConfig/mdc2020_DIOTail.ini:17: leftover conflict marker
CampaignConfig/mdc2020_DIOTail.ini:20: leftover conflict marker
CampaignConfig/mdc2020_DIOTail.ini:23: leftover conflict marker
CampaignConfig/mdc2020_celeadinglog.ini:17: leftover conflict marker
CampaignConfig/mdc2020_celeadinglog.ini:20: leftover conflict marker
CampaignConfig/mdc2020_celeadinglog.ini:23: leftover conflict marker
CampaignConfig/mdc2024_ensembles.ini:17: leftover conflict marker
CampaignConfig/mdc2024_ensembles.ini:20: leftover conflict marker
CampaignConfig/mdc2024_ensembles.ini:23: leftover conflict marker
Test Result Details
test with Command did not list any other PRs to include
merge fd1e2ef into ab19ec3 merge failed

@sophiemiddleton
Copy link
Collaborator Author

I fixed those merge conflicts, sorry for delay

Copy link
Collaborator

@brownd1978 brownd1978 left a comment

Choose a reason for hiding this comment

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

If you have a dedicated gen_primary2024 I don't see why you need to modify gen_Primary, since the only use case I can see for that mod is generating MDC2024. Am I missing something?

@sophiemiddleton
Copy link
Collaborator Author

Dave, there is an overlap....but the difference is that the gen_Primary.sh still uses gen fcl while gen_Primary2024 was part of my original work doing the fclless, so it uses jobdef. Of course gen_Primary should also be updated in the same way. gen_Primary2024....that is already in Mu2e but inside the fclless dir. Iwill remove it, in my next round I will change gen_Primary to use the fclless tools

Copy link
Collaborator

@brownd1978 brownd1978 left a comment

Choose a reason for hiding this comment

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

Some minor changes requested. This PR breaks a lot of new ground and is fine as a development point but it also raises long-term questions that need to be addressed in a more structural way. Those should be made into issues. I'm also requesting rbonventre to review the python scripts.

@@ -131,7 +146,7 @@ def cry_offspill_normalization(livetime, run_mode = '1BB'):
if(run_mode == '2BB'):
# 2BB
offspill_dutyfactor = 0.246
Copy link
Collaborator

Choose a reason for hiding this comment

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

here too we need a central location for common numbers. Analyzers will need to know these as well. It should become an issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes agreed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@brownd1978 brownd1978 requested a review from bonventre July 12, 2024 19:14
@sophiemiddleton
Copy link
Collaborator Author

I will remove the example fcl since it seems to be causing issues.

@sophiemiddleton
Copy link
Collaborator Author

Hi Dave, I will be away at ICHEP until 26th July following the review. I will get back to this PR when I return.

@sophiemiddleton
Copy link
Collaborator Author

@brownd1978 @bonventre I have carried out Dave's requested changes...Richie did you have any further suggestions. I will draft out a table design and send it to you.

I will soon put in another PR with changes for the RPC campaign

@brownd1978
Copy link
Collaborator

@FNALbuild run build test

@FNALbuild
Copy link
Collaborator

⌛ The following tests have been triggered for 1681fb0: build (Build queue is empty)

@FNALbuild
Copy link
Collaborator

☀️ The build tests passed at 1681fb0.

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

N.B. These results were obtained from a build of this Pull Request at 1681fb0 after being merged into the base branch at 18ba4b9.

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.

@sophiemiddleton
Copy link
Collaborator Author

Any updates on merging this?

Copy link
Collaborator

@brownd1978 brownd1978 left a comment

Choose a reason for hiding this comment

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

thanks for making the updates

@brownd1978
Copy link
Collaborator

@FNALbuild run build test

@FNALbuild
Copy link
Collaborator

⌛ The following tests have been triggered for caa6f9e: build (Build queue is empty)

@FNALbuild
Copy link
Collaborator

☀️ The build tests passed at caa6f9e.

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

N.B. These results were obtained from a build of this Pull Request at caa6f9e after being merged into the base branch at 18ba4b9.

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 brownd1978 merged commit 412ca44 into main Aug 5, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants