cms-analysis / HiggsAnalysis-CombinedLimit

CMS Higgs Combination toolkit.
https://cms-analysis.github.io/HiggsAnalysis-CombinedLimit/latest
Apache License 2.0
75 stars 381 forks source link

nckw alternative attempt to fix dataset from alternative file #832

Closed nucleosynthesis closed 1 year ago

nucleosynthesis commented 1 year ago

Attempt to fix dataset name option

Distinguish between --dataset and --dataMapName for combine and text2workspace respectively.

--dataset (or -D) allows the user to specify an alternative dataset (eg a toy) to run at the command line while --dataMapName changes the default (data_obs) dataset name in the datacard mapping name.

The latter is not passed to text2workspace via command line now when running with datacard as command line input to combine (hence why this fixes the issue) so the user would need to pass via options. Not clear if this is used ever however.

nucleosynthesis commented 1 year ago

Attempt to resolve issue #828

anigamova commented 1 year ago

So since this PR also includes changes from https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/pull/817 we should be prepared to release a revision if we merge this one, right @kcormi?

kcormi commented 1 year ago

So since this PR also includes changes from #817 we should be prepared to release a revision if we merge this one, right @kcormi?

Yes, I guess we should decide what we'd like to have in a minor release and then make sure we update the documentation as nick mentioned in #817.

nucleosynthesis commented 1 year ago

Sorry guys,

My fault that I let these changes slip in this PR. Let me know if you prefer me to revert those changes to the docs here and separate out the intended ones.

Nick

On Wed, 29 Mar 2023, 15:50 kcormi, @.***> wrote:

So since this PR also includes changes from #817 https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/pull/817 we should be prepared to release a revision if we merge this one, right @kcormi https://github.com/kcormi?

Yes, I guess we should decide what we'd like to have in a minor release and then make sure we update the documentation as nick mentioned in #817 https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/pull/817.

— Reply to this email directly, view it on GitHub https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/pull/832#issuecomment-1488774999, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMEVWZT6JN5GH6LX3GIKYDW6RD33ANCNFSM6AAAAAAWJOTYK4 . You are receiving this because you authored the thread.Message ID: @.***>

kcormi commented 1 year ago

No problem Nick,

I don't know how you have historically decided when to do new releases, but I think we could just do a minor one soon. Then it is easy enough to either merge #817 first, or just close it and only merge this one. So I don't think its an issue.

Cheers,

Kyle