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

Do we use PROOF? #43975

Closed
iarspider opened this issue Feb 15, 2024 · 18 comments
Closed

Do we use PROOF? #43975

iarspider opened this issue Feb 15, 2024 · 18 comments

Comments

@iarspider
Copy link
Contributor

In root-project/root#14513, PROOF was made optional (off by default), causing test failures when testing new ROOT master version: link.

However, it is only used by two tests:
FWCore/TFWLiteSelector/test/proof_thing_sel.C and FWCore/TFWLiteSelector/test/proof_thing2_sel.C.

Should we explicitly enable PROOF, or should we drop these two tests?

@iarspider
Copy link
Contributor Author

assign core

@cmsbuild
Copy link
Contributor

New categories assigned: core

@Dr15Jones,@makortel,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks

@iarspider
Copy link
Contributor Author

type root

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 15, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

A new Issue was created by @iarspider .

@makortel, @smuzaffar, @sextonkennedy, @rappoccio, @Dr15Jones, @antoniovilela can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@cmsbuild cmsbuild added the root label Feb 15, 2024
@Dr15Jones
Copy link
Contributor

I'm OK with dropping PROOF support. I don't know of anyone who ever used our extra code built on top of PROOF.

@dan131riley
Copy link

I agree with dropping it, I think PROOF has been passed by in favor of other developments.

@makortel
Copy link
Contributor

makortel commented Feb 15, 2024

I'm fine with dropping it, but maybe we should announce it somewhere? (just to make sure)

@smuzaffar
Copy link
Contributor

I'm fine with dropping it, but maybe we should announce it somewhere? (just to make sure)

agree. Is announcing it on SW development forum enough?

@makortel
Copy link
Contributor

I'm fine with dropping it, but maybe we should announce it somewhere? (just to make sure)

agree. Is announcing it on SW development forum enough?

We could hope so :) (i.e. I don't have a better forum to suggest)

iarspider added a commit to iarspider-cmssw/cmssw that referenced this issue Feb 16, 2024
cmsbuild added a commit that referenced this issue Feb 18, 2024
@makortel
Copy link
Contributor

makortel commented Feb 19, 2024

PROOF tests were removed in #43996

@makortel
Copy link
Contributor

@makortel
Copy link
Contributor

Do we already have a cmsdist PR that drops the PROOF from ROOT?

@iarspider
Copy link
Contributor Author

@makortel kind of: cms-sw/cmsdist#8999 . Or should I make PR(s) to explicitly disable PROOF in branches where it is enabled by default?

@makortel
Copy link
Contributor

No, I think the cmsdist PR of ROOT update (where they changed the default) does the job (and follows what your announcement message). I just wanted to record the cmsdist PR here.

@makortel
Copy link
Contributor

+core

@makortel
Copy link
Contributor

@cmsbuild, please close

@cmsbuild
Copy link
Contributor

This issue is fully signed and ready to be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants