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.

Sign up for GitHub

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

Jump to bottom

Add support for microarchitecture #12270

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add support for microarchitecture #12270

wants to merge 2 commits into from

Conversation

khurtado
Copy link
Contributor

@khurtado khurtado commented Feb 21, 2025 •
edited
Loading

Fixes #12168

Status

Ready

Description

Adds minimum microarchitecture classad based on Tag Collector information.
For x86 architecture, default to "2" when information not in Tag Collector.
When non-x86 arch in list of required architectures, return "0" instead

Is it backward compatible (if not, which system it affects?)

YES

Related PRs

None

External dependencies / deployment changes

Tag Collector URL

@dmwm-bot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 1 new failures
    • 4 changes in unstable tests
  • Python3 Pylint check: failed
    • 5 warnings and errors that must be fixed
    • 4 warnings
    • 70 comments to review
  • Pycodestyle check: succeeded
    • 25 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/381/artifact/artifacts/PullRequestReport.html

@khurtado khurtado force-pushed the microarch branch 2 times, most recently from f8de558 to 5808446 Compare February 21, 2025 20:38
@dmwm-bot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 1 new failures
    • 1 tests no longer failing
    • 3 changes in unstable tests
  • Python3 Pylint check: failed
    • 1 warnings and errors that must be fixed
    • 4 warnings
    • 70 comments to review
  • Pycodestyle check: succeeded
    • 25 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/383/artifact/artifacts/PullRequestReport.html

@dmwm-bot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 1 new failures
    • 2 changes in unstable tests
  • Python3 Pylint check: failed
    • 1 warnings and errors that must be fixed
    • 4 warnings
    • 70 comments to review
  • Pycodestyle check: succeeded
    • 25 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/384/artifact/artifacts/PullRequestReport.html

@dmwm-bot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 1 new failures
    • 3 changes in unstable tests
  • Python3 Pylint check: succeeded
    • 4 warnings
    • 70 comments to review
  • Pycodestyle check: succeeded
    • 25 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/385/artifact/artifacts/PullRequestReport.html

@dmwm-bot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 3 changes in unstable tests
  • Python3 Pylint check: succeeded
    • 4 warnings
    • 69 comments to review
  • Pycodestyle check: succeeded
    • 25 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/386/artifact/artifacts/PullRequestReport.html

@khurtado khurtado mentioned this pull request Feb 21, 2025
Open
@dmwm-bot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 1 new failures
    • 1 tests no longer failing
    • 1 changes in unstable tests
  • Python3 Pylint check: succeeded
    • 4 warnings
    • 69 comments to review
  • Pycodestyle check: succeeded
    • 25 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/387/artifact/artifacts/PullRequestReport.html

@dmwm-bot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 2 new failures
    • 1 changes in unstable tests
  • Python3 Pylint check: succeeded
    • 4 warnings
    • 69 comments to review
  • Pycodestyle check: succeeded
    • 25 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/389/artifact/artifacts/PullRequestReport.html

@dmwm-bot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 1 changes in unstable tests
  • Python3 Pylint check: succeeded
    • 4 warnings
    • 69 comments to review
  • Pycodestyle check: succeeded
    • 25 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/393/artifact/artifacts/PullRequestReport.html

@khurtado
Copy link
Contributor Author

Tested with TaskCHain and StepChain:

TC:

$ condor_q -af:h REQUIRED_MINIMUM_MICROARCH CMSSW_Versions
REQUIRED_MINIMUM_MICROARCH CMSSW_Versions
3                          CMSSW_15_0_0_pre3

SC:

$ condor_q -af:h REQUIRED_MINIMUM_MICROARCH CMSSW_Versions
REQUIRED_MINIMUM_MICROARCH CMSSW_Versions
3                          CMSSW_12_4_0_pre2,CMSSW_12_4_0_pre2,CMSSW_15_0_0_pre3

@khurtado khurtado requested a review from amaltaro February 25, 2025 17:36
@khurtado
Copy link
Contributor Author

@amaltaro this is ready for review. The code is working according to this logic:

#12168 (comment)

If there is anything that you don't agree with the logic above please let me know.

@smuzaffar
Copy link

smuzaffar commented Feb 25, 2025 •
edited
Loading

@khurtado , what do you get when you run it for CMSSW_12_4_0_pre2,CMSSW_12_4_0_pre3 only ( i.e. releases which do not have any default micro arch set)?

@khurtado
Copy link
Contributor Author

@smuzaffar I get 2.

>>> from WMCore.Services.TagCollector.TagCollector import TagCollector
>>> tc = TagCollector()
>>> tc.getGreaterMicroarchVersionNumber("CMSSW_12_4_0_pre2,CMSSW_12_4_0_pre2,CMSSW_15_0_0_pre3")
3
>>> tc.getGreaterMicroarchVersionNumber("CMSSW_12_4_0_pre2,CMSSW_12_4_0_pre3")
2

@smuzaffar
Copy link

are there any resources which do not support even v2 and do we want to use those for old release cycles e.g. CMSSW_10_6 etc?

@khurtado
Copy link
Contributor Author

khurtado commented Feb 25, 2025 •
edited
Loading

are there any resources which do not support even v2 and do we want to use those for old release cycles e.g. CMSSW_10_6 etc?

According to the comment below, only T3_US_UMiss. I can change the default to 0 (no constraint) rather than 2 if that's desired (like it is done with non-x86 architectures).

#12168 (comment)

But I think the ideal solution would be to add default michroarchitecture values for all releases. That way, the version is in control of whoever controls the XML.

@smuzaffar
Copy link

@khurtado , as non-x86-64 releases do not have default_micro_arch and you use 0, so I would suggest that do the same for all releases. If no default mixcro arch is set then use 0 otherwise use the highest

amaltaro
amaltaro requested changes Feb 26, 2025
View reviewed changes
Copy link
Contributor

@amaltaro amaltaro left a comment

Choose a reason for hiding this comment

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

@khurtado thank you for providing these changes. Please find some comments along the code.

@dmwm-bot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 1 new failures
    • 1 changes in unstable tests
  • Python3 Pylint check: succeeded
    • 4 warnings
    • 77 comments to review
  • Pycodestyle check: succeeded
    • 25 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/397/artifact/artifacts/PullRequestReport.html

@dmwm-bot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 1 new failures
    • 1 changes in unstable tests
  • Python3 Pylint check: succeeded
    • 4 warnings
    • 77 comments to review
  • Pycodestyle check: succeeded
    • 25 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/398/artifact/artifacts/PullRequestReport.html

@dmwm-bot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 2 changes in unstable tests
  • Python3 Pylint check: succeeded
    • 4 warnings
    • 78 comments to review
  • Pycodestyle check: succeeded
    • 25 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/400/artifact/artifacts/PullRequestReport.html

@khurtado
Copy link
Contributor Author

@smuzaffar Ok, I have changed the default to 0
@amaltaro I updates the unit tests to include these 2 new methods.

@dmwm-bot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 1 tests no longer failing
    • 3 changes in unstable tests
  • Python3 Pylint check: succeeded
    • 4 warnings
    • 78 comments to review
  • Pycodestyle check: succeeded
    • 25 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/412/artifact/artifacts/PullRequestReport.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amaltaro amaltaro amaltaro requested changes

@smuzaffar smuzaffar smuzaffar left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CMSSW 15 won't support microarch-v2
4 participants
@khurtado @dmwm-bot @smuzaffar @amaltaro