search

Mu2e / Offline

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

Simulation updates to include and fix STM #1278

Closed PawelPlesniak closed 3 months ago

PawelPlesniak commented 4 months ago

Files required to perform STM simulation studies from EleBeamCat and MuBeamCat datasets to STMDet. Summarizing

FNALbuild commented 4 months ago

Hi @PawelPlesniak, 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.

The following users requested to be notified about changes to these packages: @resnegfk

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

About FNALbuild. Code review on Mu2e/Offline.

FNALbuild commented 4 months ago

:umbrella: The build tests failed for c4687f8b96038d30c3ea7263cfd84c4872f3fcc6.

Test Result Details
test with :white_check_mark: Command did not list any other PRs to include
merge :white_check_mark: Merged c4687f8b96038d30c3ea7263cfd84c4872f3fcc6 at ee4ce2c72021c785098852071cdb5ef741977a15
build (prof) :white_check_mark: Log file. Build time: 11 min 15 sec
ceSimReco :stopwatch: :x: Log file. Timed out.
g4test_03MT :x: Log file. Return Code 134.
transportOnly :x: Log file. Return Code 134.
POT :x: Log file. Return Code 134.
g4study :x: Log file. Return Code 134.
cosmicSimReco :stopwatch: :x: Log file. Timed out.
cosmicOffSpill :stopwatch: :x: Log file. Timed out.
ceSteps :x: Log file. Return Code 134.
ceDigi :stopwatch: :x: Log file. Timed out.
muDauSteps :x: Log file. Return Code 134.
ceMix :stopwatch: :x: Log file. Timed out.
rootOverlaps :x: Log file. Return Code 134.
g4surfaceCheck :x: Log file. Return Code 134.
FIXME, TODO :large_orange_diamond: TODO (5) FIXME (4) in 14 files
clang-tidy :large_orange_diamond: 2 errors 2897 warnings
whitespace check :white_check_mark: no whitespace errors found

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

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.

kutschke commented 4 months ago

@PawelPlesniak there are known issues with our cached db server. Very likely this is the source of the crashes in your jobs. Since we are not a running experiment we only have 5/8 support - we sometimes get lucky and people check email or tickets over the weekend.

kutschke commented 4 months ago

Let's see if the return code 134 are transient. The DB problems are still present so the timeouts should ermain.

@FNALbuild run build test

FNALbuild commented 4 months ago

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

FNALbuild commented 4 months ago

:memo: The HEAD of main has changed to 0855ab6c084e91f71b8d07271c8473f6788e8825. Tests are now out of date.

FNALbuild commented 4 months ago

:umbrella: The build tests failed for c4687f8b96038d30c3ea7263cfd84c4872f3fcc6.

Test Result Details
test with :white_check_mark: Command did not list any other PRs to include
merge :white_check_mark: Merged c4687f8b96038d30c3ea7263cfd84c4872f3fcc6 at ee4ce2c72021c785098852071cdb5ef741977a15
build (prof) :white_check_mark: Log file. Build time: 11 min 02 sec
ceSimReco :stopwatch: :x: Log file. Timed out.
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 :stopwatch: :x: Log file. Timed out.
cosmicOffSpill :stopwatch: :x: Log file. Timed out.
ceSteps :white_check_mark: Log file.
ceDigi :stopwatch: :x: Log file. Timed out.
muDauSteps :white_check_mark: Log file.
ceMix :stopwatch: :x: Log file. Timed out.
rootOverlaps :white_check_mark: Log file.
g4surfaceCheck :white_check_mark: Log file.
FIXME, TODO :large_orange_diamond: TODO (5) FIXME (4) in 14 files
clang-tidy :large_orange_diamond: 2 errors 2897 warnings
whitespace check :white_check_mark: no whitespace errors found

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

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.

kutschke commented 4 months ago

The DB cache server is back up.

@FNALbuild run build test

kutschke commented 4 months ago

@PawelPlesniak Please make a trivial commit - change a comment or something. I think the CI is too smart for it's own good and will not let me repeat a CI unless something changed.

kutschke commented 4 months ago

@PawelPlesniak Please make a trivial commit - change a comment or something. I think the CI is too smart for it's own good and will not let me repeat a CI unless something changed.

I think you can skip this. There has been an intervening merge so CI should be unblocked.

kutschke commented 4 months ago

@FNALbuild run build test

FNALbuild commented 4 months ago

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

FNALbuild commented 4 months ago

:sunny: The build tests passed at c4687f8b96038d30c3ea7263cfd84c4872f3fcc6.

Test Result Details
test with :white_check_mark: Command did not list any other PRs to include
merge :white_check_mark: Merged c4687f8b96038d30c3ea7263cfd84c4872f3fcc6 at fc6b3b1e954528ac4f5f8b3f0ad965633dc79e64
build (prof) :white_check_mark: Log file. Build time: 10 min 56 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 :large_orange_diamond: TODO (5) FIXME (4) in 14 files
clang-tidy :large_orange_diamond: 2 errors 2897 warnings
whitespace check :white_check_mark: no whitespace errors found

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

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.

PawelPlesniak commented 4 months ago

Thanks for starting to take a look Rob, I have made the suggested corrections and reintroduced the missing module. Once you have some time to look at the code I will correct the other changes, likely this weekend. Cheers :)

kutschke commented 3 months ago

@FNALbuild run build test

FNALbuild commented 3 months ago

:hourglass: The following tests have been triggered for 2d72375aa252f6c1e9dad9edd067e5cb5a7aba4c: build (Build queue has 2 jobs)

FNALbuild commented 3 months ago

:sunny: The build tests passed at 2d72375aa252f6c1e9dad9edd067e5cb5a7aba4c.

Test Result Details
test with :white_check_mark: Command did not list any other PRs to include
merge :white_check_mark: Merged 2d72375aa252f6c1e9dad9edd067e5cb5a7aba4c at c33c86d91df15f58c1c33f5477f58d4b636b57f0
build (prof) :white_check_mark: Log file. Build time: 11 min 07 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 :large_orange_diamond: TODO (5) FIXME (4) in 14 files
clang-tidy :large_orange_diamond: 2 errors 2921 warnings
whitespace check :white_check_mark: no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 2d72375aa252f6c1e9dad9edd067e5cb5a7aba4c after being merged into the base branch at c33c86d91df15f58c1c33f5477f58d4b636b57f0.

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 3 months ago

The syntax of std::optional is that it returns a unique_ptr to the object, not the object itself, hence the need for *. I suspect the reason It doesn’t work as an initializer is that the call isn’t exception-safe, ie it could throw, which is not supported in initialization.

hasValue is deprecated syntax for optional parameters. Current best practice is to obtain the (optional) value from the parameter and test that:

auto verbose = conf().verbose(); if(verbose)_verbose = *verbose;

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

On Tue, Jun 11, 2024 at 01:03 Pawel Plesniak @.***> wrote:

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

In Analyses/src/CountVDHits_module.cc https://github.com/Mu2e/Offline/pull/1278#discussion_r1634389198:

  • _conf(conf()),
  • _stepPointMCsToken(consumes(conf().stepPointMCsTag())),
  • _enabledVDs(conf().enabledVDs())
  • {
  • // Sort all elements to be in ascending order
  • std::sort(_enabledVDs.begin(), _enabledVDs.end());
  • // Remove all duplicate elements
  • std::vector::iterator _uniqueIndex = std::unique(_enabledVDs.begin(), _enabledVDs.end());
  • // Removed undefined memory
  • _enabledVDs.resize(std::distance(_enabledVDs.begin(), _uniqueIndex));
  • // Insert _enabledVDs.size() zeros into the counter vector
  • _enabledVDStepPointMCsCount.insert(_enabledVDStepPointMCsCount.end(), _enabledVDs.size(), 0);
  • // If verbose has been provided, override the local variable
  • if (conf().verbose.hasValue()){
  • _verbose = *std::move(conf().verbose());

I had tried this before using std::move, and it continues to return the same error: error: cannot convert 'std::optional' to 'bool' in assignment but *std::move(conf().verbose()) works. It does work in the member initialization list, but not in the constructor definition inside {}. Do you know why it would fail in one scope and not the other?

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

kutschke commented 3 months ago

The syntax of std::optional is that it returns a unique_ptr to the object, not the object itself, hence the need for *. I suspect the reason It doesn’t work as an initializer is that the call isn’t exception-safe, ie it could throw, which is not supported in initialization. hasValue is deprecated syntax for optional parameters.

I had missed the fact that it was an optional Parameter.

PawelPlesniak commented 3 months ago

@FNALbuild run build test

FNALbuild commented 3 months ago

:hourglass: The following tests have been triggered for 93a36a47d27180f682bcacd7459f0f59d20ed742: build (Build queue has 6 jobs)

FNALbuild commented 3 months ago

:sunny: The build tests passed at 93a36a47d27180f682bcacd7459f0f59d20ed742.

Test Result Details
test with :white_check_mark: Command did not list any other PRs to include
merge :white_check_mark: Merged 93a36a47d27180f682bcacd7459f0f59d20ed742 at c33c86d91df15f58c1c33f5477f58d4b636b57f0
build (prof) :white_check_mark: Log file. Build time: 11 min 25 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 :large_orange_diamond: TODO (5) FIXME (4) in 14 files
clang-tidy :large_orange_diamond: 2 errors 2920 warnings
whitespace check :white_check_mark: no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 93a36a47d27180f682bcacd7459f0f59d20ed742 after being merged into the base branch at c33c86d91df15f58c1c33f5477f58d4b636b57f0.

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.

PawelPlesniak commented 3 months ago

Thanks for the feedback Rob, I've looked through the code and there was only code in Analyses/src/CountVDHits_module.cc which was replaced with the messagefacility. The code in the STMMC directory will still be developed to include other functionality, primarily to do with timestamping. I've left it as code in STMMC/src/HPGeWaveformsFromGeantSim_module.cc, otherwise has been removed.

If there's anything else that I can change let me know :)

PawelPlesniak commented 3 months ago

@FNALbuild run build test

FNALbuild commented 3 months ago

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

FNALbuild commented 3 months ago

:sunny: The build tests passed at b5514ed705f3f1ab32f92ad01ffc44e73713f569.

Test Result Details
test with :white_check_mark: Command did not list any other PRs to include
merge :white_check_mark: Merged b5514ed705f3f1ab32f92ad01ffc44e73713f569 at c33c86d91df15f58c1c33f5477f58d4b636b57f0
build (prof) :white_check_mark: Log file. Build time: 11 min 11 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 :large_orange_diamond: TODO (5) FIXME (4) in 14 files
clang-tidy :large_orange_diamond: 2 errors 2920 warnings
whitespace check :white_check_mark: no whitespace errors found

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

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.