TopEFT / topeft

15 stars 24 forks source link

edited condor datacard making command to remove do-nuisance flag #353

Closed abasnet97 closed 1 year ago

abasnet97 commented 1 year ago

This PR fixes the issue with the --do-nuisance flag not working properly when running datacard maker (make_cards.py) using condor. The issue was that even when the flag was not explicitly called, the resulting datacards would still have the systematics included. It turned out that the datacard making command passed to condor had --do-nuisance flag always included. Getting rid of that and adding a new option to use the flag in run_condor method resolved the issue.

kmohrman commented 1 year ago

Thanks very much for fixing this! Just wanted to check that you've verified that these changes did not inadvertently cause any changes when running without condor (i.e. explicitly verified that including or excluding the --do-nuisance flag now has the expected effect both without condor and with condor?)

abasnet97 commented 1 year ago

Yes. I have double-checked that. I ran the script using condor without passing the flag, and I see that the resulting datacard has no systematics included. Again, ran the script using condor with the flag included, and the resulting datcard has systematics included.

UPDATE: Totally forgot that Kelci also asked if this has any inadvertent impact while running the datacard maker locally. I made sure to check that these changes have no impact whatsoever when running the script locally.

codecov[bot] commented 1 year ago

Codecov Report

Merging #353 (20c5a5a) into technical_improvements (c186e62) will decrease coverage by 0.02%. The diff coverage is 0.00%.

@@                    Coverage Diff                     @@
##           technical_improvements     #353      +/-   ##
==========================================================
- Coverage                   33.90%   33.89%   -0.02%     
==========================================================
  Files                          40       40              
  Lines                        6742     6742              
==========================================================
- Hits                         2286     2285       -1     
- Misses                       4456     4457       +1     
Flag Coverage Δ
unittests 33.89% <0.00%> (-0.02%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
analysis/topEFT/make_cards.py 57.64% <0.00%> (-0.59%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

kmohrman commented 1 year ago

Thanks again for this PR Aashwin. Since it looks like all of the comments have been addressed, I'd be happy to go ahead and merge it now.