Mu2e / Offline

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

Loop dependencies in fcl files #530

Closed kutschke closed 2 years ago

kutschke commented 3 years ago

The following .fcl files in Offline contain references to Production/

./Analyses/test/Mu2eCCFC_conversion.fcl ./Analyses/test/Mu2eCCFC_dio.fcl ./Mu2eKinKal/test/SeedTest.fcl ./TEveEventDisplay/CallerFclExamples/POT.fcl ./Validation/test/ceDigi.fcl ./Validation/test/ceMix.fcl ./Validation/test/ceSteps.fcl ./Validation/test/muDauSteps.fcl

We need to understand where to move these to undo the loop dependency.

Do we want to allow loop dependencies in clearly marked directories that include the string "test"? We have done that for some other policies.

rlcee commented 3 years ago

The Validation files are obsolete, those were moved to Production. I'll delete them

kutschke commented 3 years ago

The Validation files are obsolete, those were moved to Production. I'll delete them

Thanks Ray. I will talk with Sophie about TEveEventDisplay and with Bob about the two in Analysis.

Dave: can look at the file in Mu2eKinKal.

rlcee commented 3 years ago

Validation files removed in #531

brownd1978 commented 3 years ago

I thought we agreed to not consider /test contents as subject to this loop test. The KinKal dependence is not a real (circular) loop.

If people are really bothered by this the /test content can be migrated to Production/Tests. I don't think it solves any actual problems though. Dave

On Thu, Jul 15, 2021 at 8:27 AM Rob Kutschke @.***> wrote:

The following .fcl files in Offline contain references to Production/

./Analyses/test/Mu2eCCFC_conversion.fcl ./Analyses/test/Mu2eCCFC_dio.fcl ./Mu2eKinKal/test/SeedTest.fcl ./TEveEventDisplay/CallerFclExamples/POT.fcl ./Validation/test/ceDigi.fcl ./Validation/test/ceMix.fcl ./Validation/test/ceSteps.fcl ./Validation/test/muDauSteps.fcl

We need to understand where to move these to undo the loop dependency.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Mu2e/Offline/issues/530, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH57YZGL2LGP7N6ZO7XHTTX347FANCNFSM5ANZGL7A .

-- David Nathan Brown @.*** Office Phone (510) 486-7261 Fax 495-2957 Lawrence Berkeley National Lab MS 50R5008 (50-6026C) Berkeley, CA 94720

kutschke commented 3 years ago

After the email that you saw I added a comment about test directories. I am Ok sticking with that policy for cases where it's thoughtfully considered and it's the right place for it. It sounds like the KinKal case satsifies this.

rlcee commented 3 years ago

I would vote to organize the dependencies logically, i.e. no reference to Production in Offline. It's just a bad example.

brownd1978 commented 3 years ago

I propose to create a Tests repository, which can reference both Offline and Production. I would make it open to anyone who wants to commit to it, with no 'code review'. We should use it only for non-essential test code. It will keep us from having to update Offline everytime some thinks of a new test plot. I would move everything in TrkDiag/tests into Tests/Tracking. What do you think?

Dave

On Fri, Jul 16, 2021 at 5:19 PM Ray Culbertson @.***> wrote:

I would vote to organize the dependencies logically, i.e. no reference to Production in Offline. It's just a bad example.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Mu2e/Offline/issues/530#issuecomment-881781988, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH573ZUI4AVJFIKQGY7MTTYDD75ANCNFSM5ANZGL7A .

-- David Nathan Brown @.*** Office Phone (510) 486-7261 Fax 495-2957 Lawrence Berkeley National Lab MS 50R5008 (50-6026C) Berkeley, CA 94720

rlcee commented 3 years ago

I think creating new repos that depend on Production and Offline is the logical conclusion of

I'd only pause to think about what other fcl might belong here, like Production/Validation, JobConfig/test or validation. Should it then be called Validation? There are more examples of this category coming, like JobConfig/alignment, event displays, calibration or ntuple making, etc. Really anything that might need to be aware of the reco path, or might need to generate sim samples.

Another observation: the current dependency order is

brownd1978 commented 3 years ago

I think the preconditions for 'Test' preclude any overlap with Production ( Validation or JobConfig). Ff a 'test' is really used to generate a prodition (say) then it belongs in the Production repo.

I think we've converged on the concept of Tests repository. I propose to structure it as Tests/Tracking, etc, and content from TrkDiag and Mu2eKinKal there now. Do we agree? If so I'll create it today.

On Sat, Jul 17, 2021 at 8:58 AM Ray Culbertson @.***> wrote:

I think creating new repos that depend on Production and Offline is the logical conclusion of

  • they have these dependencies
  • they don't really belong in Production I'd only pause to think about what other fcl might belong here, like Production/Validation, JobConfig/test or validation. Should it then be called Validation? There are more examples of this category coming, like JobConfig/alignment, event displays, calibration or ntuple making, etc. Really anything that might need to be aware of the reco path, or might need to generate sim samples.

Another observation: the current dependency order is

  • Tutorial TrackerAlignment TrkAna Production Offline So really Production/alignment could go back in TrackerAlignment, which feels very right. Event displays and ntuple makers could be handled the same way.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Mu2e/Offline/issues/530#issuecomment-881918964, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH5765VBO2LBQ6EWAECWDTYGSBDANCNFSM5ANZGL7A .

-- David Nathan Brown @.*** Office Phone (510) 486-7261 Fax 495-2957 Lawrence Berkeley National Lab MS 50R5008 (50-6026C) Berkeley, CA 94720

kutschke commented 3 years ago

Hi Dave,

I am not 100% certain that we will end up with this but let’s start and see where it takes us.

Rob

From: David Brown @.> Reply-To: Mu2e/Offline @.> Date: Monday, July 19, 2021 at 1:20 PM To: Mu2e/Offline @.> Cc: Rob Kutschke @.>, Author @.***> Subject: Re: [Mu2e/Offline] Loop dependencies in fcl files (#530)

I think the preconditions for 'Test' preclude any overlap with Production ( Validation or JobConfig). Ff a 'test' is really used to generate a prodition (say) then it belongs in the Production repo.

I think we've converged on the concept of Tests repository. I propose to structure it as Tests/Tracking, etc, and content from TrkDiag and Mu2eKinKal there now. Do we agree? If so I'll create it today.

On Sat, Jul 17, 2021 at 8:58 AM Ray Culbertson @.***> wrote:

I think creating new repos that depend on Production and Offline is the logical conclusion of

  • they have these dependencies
  • they don't really belong in Production I'd only pause to think about what other fcl might belong here, like Production/Validation, JobConfig/test or validation. Should it then be called Validation? There are more examples of this category coming, like JobConfig/alignment, event displays, calibration or ntuple making, etc. Really anything that might need to be aware of the reco path, or might need to generate sim samples.

Another observation: the current dependency order is

  • Tutorial TrackerAlignment TrkAna Production Offline So really Production/alignment could go back in TrackerAlignment, which feels very right. Event displays and ntuple makers could be handled the same way.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Mu2e/Offline/issues/530#issuecomment-881918964https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Mu2e_Offline_issues_530-23issuecomment-2D881918964-253E&d=DwQFaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=Zi5IH-k2_JiK_Wipiv3pwwSXO9UNaHiLP8sisfPz__k&m=W_f1rM0xonnS8jeTbBM8D_iFvsNKMQXxgATXPcbdLQY&s=7pri1ekGJ6ADmmW-Fu68J-TgTTrCsGlpgQqHK_O-XO0&e=, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH5765VBO2LBQ6EWAECWDTYGSBDANCNFSM5ANZGL7Ahttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_ABAH5765VBO2LBQ6EWAECWDTYGSBDANCNFSM5ANZGL7A-253E&d=DwQFaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=Zi5IH-k2_JiK_Wipiv3pwwSXO9UNaHiLP8sisfPz__k&m=W_f1rM0xonnS8jeTbBM8D_iFvsNKMQXxgATXPcbdLQY&s=bfpq23Lr758DrxrxxpTtTYDU3xS9xrv-9v8yOzpVuFg&e= .

-- David Nathan Brown @.*** Office Phone (510) 486-7261 Fax 495-2957 Lawrence Berkeley National Lab MS 50R5008 (50-6026C) Berkeley, CA 94720

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Mu2e_Offline_issues_530-23issuecomment-2D882753484&d=DwMFaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=Zi5IH-k2_JiK_Wipiv3pwwSXO9UNaHiLP8sisfPz__k&m=W_f1rM0xonnS8jeTbBM8D_iFvsNKMQXxgATXPcbdLQY&s=jz3206N2kYhYcYrzVnJdwiSWBaVYOuMF-gDB5LqUs6w&e=, or unsubscribehttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_ABHY5ZQIWK4PHI5XH65UQ2TTYRS7RANCNFSM5ANZGL7A&d=DwMFaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=Zi5IH-k2_JiK_Wipiv3pwwSXO9UNaHiLP8sisfPz__k&m=W_f1rM0xonnS8jeTbBM8D_iFvsNKMQXxgATXPcbdLQY&s=BrH1l9c2AXztR5aAb7hpSwZV87pf6c3uHy9VllBCwcg&e=.

rlcee commented 3 years ago

I think the preconditions for 'Test' preclude any overlap with Production ( Validation or JobConfig).

Are you referring to the suggestion that Production/Validation be moved to test? Why not move it? Tests is a better name for a repo of validation than Production. But I don't feel strongly.

If a 'test' is really used to generate a prodition (say) then it belongs in the Production repo.

Are you referring to my suggestion to move Production/JobConfig/alignment back to TrackerConditions? It sounds like you are proposing a rule that all Pass1 fcl is in Production. That sounds reasonable, but it is actually still breaking the dependency order. That fcl in Production does refer to prologs and modules in TrackerAlignment, which is above Production in the dependency order, making a loop. You could imagine putting TrackerAlignment below Production in the dependency order, but then everyone who uses Production has to be aware of TrackerAlignment.

As far as I am concerned, you can create and populate Test. I don't feel strongly where Validation ends up. I do feel strongly about the TrackerAlignment issue.

brownd1978 commented 3 years ago

On Mon, Jul 19, 2021 at 1:33 PM Ray Culbertson @.***> wrote:

I think the preconditions for 'Test' preclude any overlap with Production ( Validation or JobConfig).

Are you referring to the suggestion that Production/Validation be moved to test? Why not move it? Tests is a better name for a repo of validation than Production. But I don't feel strongly.

I agree it is more logical to have Validation in Tests. If you'd like to move it after Tests is created thats fine with me.

If a 'test' is really used to generate a prodition (say) then it belongs in the Production repo.

Are you referring to my suggestion to move Production/JobConfig/alignment back to TrackerConditions? It sounds like you are proposing a rule that all Pass1 fcl is in Production. That sounds reasonable, but it is actually still breaking the dependency order. That fcl in Production does refer to prologs and modules in TrackerAlignment, which is above Production in the dependency order, making a loop. You could imagine putting TrackerAlignment below Production in the dependency order, but then everyone who uses Production has to be aware of TrackerAlignment.

Yes, my model is Production produces all the art files needed for all downstream consumers, including alignment. The 2 scripts currently in TrackerAlignment that source Production fcl should be split, so that Production/reco produces the art files, then the TrackerAlignment jobs run on those to produce the Millipede files. That will break the dependency. I think the split should be easy, the OffSpill and Extracted triggers includes lines for alignment, and they can be used to create an appropriate reco stream. I'll work with Richie on it.

There is also

include "Production/JobConfig/reco/misalign_epilog.fcl"

This is a campaign-specific configuration file whose content should come from Proditions. So it (and its sub-files) should be removed. Is that what you're referring to above (moving back to TrackerConditions)?

As far as I am concerned, you can create and populate Test. I don't feel strongly where Validation ends up. I do feel strongly about the TrackerAlignment issue.

The main point of Tests is to avoid tying Offline builds to updates in QC codes. I think the long-term evolution should be to move everything currently in Offline/Xxx/tests into Tests/Xxx. I'll propose this on Wednesday in the meeting.

Dave

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Mu2e/Offline/issues/530#issuecomment-882840963, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH576WCIUFUPBXINTRVALTYSD2ZANCNFSM5ANZGL7A .

-- David Nathan Brown @.*** Office Phone (510) 486-7261 Fax 495-2957 Lawrence Berkeley National Lab MS 50R5008 (50-6026C) Berkeley, CA 94720

rlcee commented 3 years ago

If I understand, your model is that TrackerAlignment and Production are peers in the dependency tree, that neither depends on the other. Production runs reco and produces a file, then TrackerAlignment code and fcl makes ntuples. This strategy which you might call "isolation by data file" is one I favor. I use it in the nightly validation jobs and I suggested it to separate EventDisplay from reco.

Some current fcl in TrackerAlignment refers to Production and some fcl in Production/JobConfig/alignment refers to TrackerAlignment. I'm not sure what each does, but both need to be dealt with in this scheme, and I assume it can be done.

In the scheme I was proposing, TrackerAlignment is above Production in the dependency order and therefore may depend on Production. The ntuple-maker can obviously live here, but optionally, if the alignment needs to modify reco, by turning off a module, or employing a module in TrackerAlignment (maybe a filter), or whatever, then that specialized reco can also live here.

Enforcing peer repos limits you, while allowing TrackerAlignment to depend on Production gives you more flexibility. In the peers scheme, if there is a modified reco module that needs to part of the alignment reco job, it would have to move from TrackerAlignment to Offline, in order for Production to access it.

Muse only understands a linear dependence order, so TrackerAlignment is/will be above Production in dependency, so it will be ready if the peers model runs into this issue.

Or maybe you are saying that the idea that pass1 can only depend on Production and Offline is the primary principle, so this discussion is moot.

brownd1978 commented 3 years ago

Talking with Richie I learned my model for splitting is too simplistic. Richie needs to iterate running reco to account for 2nd order effects, so TrackerAlignment should be above Production. We can remove the loop by moving one fcl file and maybe 1 filter. Testing isn’t possible right now as Minuit is broken in art 3_09.

We probably will want to run the module that produces the millipede data in pass 1, as that dataset is large. Do you see a problem with that? I think it just means the pass1 musing needs Offline, Production and TrackerAlignment. We also need to define a data type for the Milipede format. Dave

On Mon, Jul 19, 2021 at 21:34 Ray Culbertson @.***> wrote:

If I understand, your model is that TrackerAlignment and Production are peers in the dependency tree, that neither depends on the other. Production runs reco and produces a file, then TrackerAlignment code and fcl makes ntuples. This strategy which you might call "isolation by data file" is one I favor. I use it in the nightly validation jobs and I suggested it to separate EventDisplay from reco.

Some current fcl in TrackerAlignment refers to Production and some fcl in Production/JobConfig/alignment refers to TrackerAlignment. I'm not sure what each does, but both need to be dealt with in this scheme, and I assume it can be done.

In the scheme I was proposing, TrackerAlignment is above Production in the dependency order and therefore may depend on Production. The ntuple-maker can obviously live here, but optionally, if the alignment needs to modify reco, by turning off a module, or employing a module in TrackerAlignment (maybe a filter), or whatever, then that specialized reco can also live here.

Enforcing peer repos limits you, while allowing TrackerAlignment to depend on Production gives you more flexibility. In the peers scheme, if there is a modified reco module that needs to part of the alignment reco job, it would have to move from TrackerAlignment to Offline, in order for Production to access it.

Muse only understands a linear dependence order, so TrackerAlignment is/will be above Production in dependency, so it will be ready if the peers model runs into this issue.

Or maybe you are saying that the idea that pass1 can only depend on Production and Offline is the primary principle, so this discussion is moot.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/Mu2e/Offline/issues/530#issuecomment-883044951, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABAH572TTNTPT2YH4GZ2BJ3TYT4ETANCNFSM5ANZGL7A .

-- David Nathan Brown @.*** Office Phone (510) 486-7261 Fax 495-2957 Lawrence Berkeley National Lab MS 50R5008 (50-6026C) Berkeley, CA 94720

rlcee commented 3 years ago

Talking with Richie I learned my model for splitting is too simplistic. Richie needs to iterate running reco to account for 2nd order effects, so TrackerAlignment should be above Production. We can remove the loop by moving one fcl file and maybe 1 filter. Testing isn’t possible right now as Minuit is broken in art 3_09. We probably will want to run the module that produces the millipede data in pass 1, as that dataset is large. Do you see a problem with that?

I think this is fine

I think it just means the pass1 musing needs Offline, Production and TrackerAlignment. We also need to define a data type for the Milipede format.

Also all OK. I imagine there will be a "Pass1" Musing

kutschke commented 3 years ago

This all works for me.

From: Ray Culbertson @.> Reply-To: Mu2e/Offline @.> Date: Tuesday, July 20, 2021 at 12:07 PM To: Mu2e/Offline @.> Cc: Rob Kutschke @.>, Author @.***> Subject: Re: [Mu2e/Offline] Loop dependencies in fcl files (#530)

Talking with Richie I learned my model for splitting is too simplistic. Richie needs to iterate running reco to account for 2nd order effects, so TrackerAlignment should be above Production. We can remove the loop by moving one fcl file and maybe 1 filter. Testing isn’t possible right now as Minuit is broken in art 3_09. We probably will want to run the module that produces the millipede data in pass 1, as that dataset is large. Do you see a problem with that?

I think this is fine

I think it just means the pass1 musing needs Offline, Production and TrackerAlignment. We also need to define a data type for the Milipede format.

Also all OK. I imagine there will be a "Pass1" Musing

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Mu2e_Offline_issues_530-23issuecomment-2D883553725&d=DwMFaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=Zi5IH-k2_JiK_Wipiv3pwwSXO9UNaHiLP8sisfPz__k&m=6WO6TzmbIwbPQE6-Q2yXZjqVUhHe6MjpdvaJO8SYFvo&s=qaPHd661gFyW3rgdElXX8AxrPTgSLGNMCvWyMTKsFe4&e=, or unsubscribehttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_ABHY5ZUAZSQDUPXCPCKA6BDTYWUMNANCNFSM5ANZGL7A&d=DwMFaQ&c=gRgGjJ3BkIsb5y6s49QqsA&r=Zi5IH-k2_JiK_Wipiv3pwwSXO9UNaHiLP8sisfPz__k&m=6WO6TzmbIwbPQE6-Q2yXZjqVUhHe6MjpdvaJO8SYFvo&s=qHaqOXOd2cWlcAHhCRsZqZh3-KqFMZ-Lb_qcVysWApM&e=.

rlcee commented 2 years ago

This was finished up on #426