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

ci: Full chain ODD monitoring follow-up #1250

Merged
merged 4 commits into from
May 13, 2022

Conversation

paulgessinger
Copy link
Member

Follow-up to #1247.

This moves the extra invocation of thirdparty/OpenDataDetector/ci/full_chain_odd.py into the main CI/physmon/physmon.py script where we basically run this chain anyway. This saves some time since we can generate the residual and pulls output from that invocation. I also change the ActsAnalysisResidualsAndPulls to be called directly from the python script, so we can reuse the temporary directory from there.

This also changes ActsAnalysisResidualsAndPulls to return UNIX-style error codes (0: ok, != 0, failure)

@paulgessinger paulgessinger added this to the next milestone May 11, 2022
@paulgessinger
Copy link
Member Author

Would be great to get your opinion on this @andiwand.

The CI will likely fail right now, because the configuration from the reference might be from a subtly different configuration.

Copy link
Contributor

@andiwand andiwand left a comment

Choose a reason for hiding this comment

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

cool 👍

I am just missing the setup and call of the sequencer https://gitlab.cern.ch/acts/OpenDataDetector/-/blob/main/ci/full_chain_odd.py#L69-107. is that not necessary anymore?

@codecov
Copy link

codecov bot commented May 11, 2022

Codecov Report

Merging #1250 (b117e86) into main (4f3a157) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1250   +/-   ##
=======================================
  Coverage   47.89%   47.89%           
=======================================
  Files         375      375           
  Lines       19588    19588           
  Branches     9214     9214           
=======================================
  Hits         9382     9382           
  Misses       3822     3822           
  Partials     6384     6384           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@paulgessinger
Copy link
Member Author

cool 👍

I am just missing the setup and call of the sequencer https://gitlab.cern.ch/acts/OpenDataDetector/-/blob/main/ci/full_chain_odd.py#L69-107. is that not necessary anymore?

It should be using the one here. Or do you mean something else?

@paulgessinger
Copy link
Member Author

Ok, reference is updated, CI should run through now.

@paulgessinger
Copy link
Member Author

Seems fine now, consider approving @andiwand.

@kodiakhq kodiakhq bot merged commit 7ae1264 into acts-project:main May 13, 2022
@paulgessinger paulgessinger modified the milestones: next, v19.1.0 May 25, 2022
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.

None yet

2 participants