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

Add --maxmem_profile to all runTheMatrix workflows to log mamimum memory allocated #2185

Closed
wants to merge 2 commits into from

Conversation

gartung
Copy link
Member

@gartung gartung commented Feb 26, 2024

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @gartung for branch master.

@cmsbuild, @aandvalenzuela, @iarspider, @smuzaffar can you please review it and eventually sign? Thanks.
@antoniovilela, @rappoccio, @sextonkennedy you are the release manager for this.
cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 26, 2024

cms-bot internal usage

@gartung
Copy link
Member Author

gartung commented Feb 26, 2024

@makortel

@makortel
Copy link
Contributor

addresses makortel/framework#673

(this PR is more about cms-sw/framework-team#829)

@makortel
Copy link
Contributor

I think there are some test flavors for which this monitoring would be harmful (like UBSAN, ASAN, threading etc). Perhaps the best way to get started would be to limit to the default testing set on x86 (we could then expand after gaining more experience).

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 4, 2024

Pull request #2185 was updated.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 4, 2024

REMINDER @rappoccio, @sextonkennedy, @antoniovilela: This PR was tested with cms-sw/cmssw#44001, please check if they should be merged together

@antoniovilela
Copy link

cms-sw/cmssw#44001 has been merged.

@makortel
Copy link
Contributor

makortel commented Mar 4, 2024

Maybe we wait for Shahzad to get back to decide how exactly to continue (unless @aandvalenzuela @iarspider would know on the top of their head what exactly would need to be done to achieve #2185 (comment)).

@@ -49,6 +49,7 @@ pushd "$WORKSPACE/matrix-results"
case "${TEST_FLAVOR}" in
gpu ) MATRIX_ARGS="-w gpu ${MATRIX_ARGS}" ;;
high_stats ) CMD_OPTS="-n 500" ; MATRIX_ARGS="-i all ${MATRIX_ARGS}" ;;
maxmem_profile ) CMD_OPTS="--maxmem_profile" ;;
Copy link
Contributor

Choose a reason for hiding this comment

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

@gartung , is --maxmem_profile supported by all cmssw release cycles? If not then I would suggest to run runTheMatrix.py --help to see if this opt is supported or not. I n case a cmssw release cycle does not support it then just mark the commit status successfull with message Memory profile not run due to lack of support

Copy link
Contributor

Choose a reason for hiding this comment

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

is --maxmem_profile supported by all cmssw release cycles?

It is not. The option was added in 14_1_X by cms-sw/cmssw#44001 .

Patrick is on a leave, so someone else would have to continue from here. I could do that, but I'd need guidance how to achieve #2185 (comment) .

@smuzaffar
Copy link
Contributor

So remind me what we want to do with this change? from the change it looks like we want to run extra relvals with --maxmem_profile option e.g. one can say enable maxmem_profile and then bot should run normal relval + all normal relvals with --maxmem_profile .. right?

@makortel
Copy link
Contributor

I want all normal relval on x86 (only) to be run with --maxmem_profile (i.e. no additional "test flavor"). Any extra workflows could include the --maxmem_profile too, but for other test flavors (non-x86 architectures, different IB flavor, extra tests) for now.

@smuzaffar
Copy link
Contributor

@gartung @makortel I have opened #2202 to enable memory profiling for default workflows for production IB/arch only

@makortel
Copy link
Contributor

I think we can close this PR in favor of #2202

@makortel
Copy link
Contributor

@cmsbuild, please close

@cmsbuild cmsbuild closed this Mar 20, 2024
@gartung gartung deleted the gartung-PR-maxmem-profile branch May 7, 2024 17:20
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.

5 participants