search

SBNSoftware / sbnci

Packages, modules and scripts for the SBN continuous integration system
2 stars 4 forks source link

CAF Checks #16

Open henrylay97 opened 3 years ago

henrylay97 commented 3 years ago

Have just been fixing an error caused by the anatree config for sbnd and it occurred to me that as of yet I don't think we perform any CAF checks at all? I'm sure this is something that has been discussed but longer term this is definitely something we'd want to be checking!

cjbacchus commented 3 years ago

Yeah, there is a really nice diff_cafs tool available that can check that two CAFs are literally identical, which I think we should be making use of, but nothing has been done about hooking this up.

wesketchum commented 3 years ago

@chilge any chance of this becomes part of the standard workflow checks for creating cafs? Then could look to add on the diff_cafs too?

chilge commented 3 years ago

there's a chance. it depends on how the rollout of what we currently have goes. the same solution being developed for the fhicl checks might be able to be easily modified for using diff_cafs.

henrylay97 commented 2 years ago

Hi all,

I think I have a working set of CAF checks (as usual this is for SBND but the infrastructure should be there for someone on ICARUS to quickly add this to their configuration too) that can now be used (following the usual testing procedure). As it can operate in a similar way to our current regression test, this would fit into the standard CI which is run for every single PR and every commit to develop. I will post some CI examples on here later.

There are huge benefits to this. The obvious is that we test the CAFMaker infrastructure which is currently not covered by the CI at all (other than it compiles). The real extra benefit compared to the other stages is that in the simple inexpensive test we are able to test physics quantities without running the full validation. Thanks to the lovely diff_cafs script we will see if any value in the CAF has changed.

@chilge @cjbackhouse @wesketchum @jzennamo I have few things I’ve found whilst developing this that I would like input on from you guys:

1) Shall we get rid of the anatree tests from the standard CI? Is anyone using the anatree anymore? All these checks do is ensure that it runs, there are no value checks.

2) The production workflow is currently using cafmakerjob_sbnd_sce_genie_and_fluxwgt.fcl, I tried using this but the setting up of the multisims took so long it wasn’t viable as a quick regression test. I am currently using cafmakerjob_sbnd.fcl, are we happy with this?

3) I also realised during this that we’re currently using standard_*_sbnd.fcl for the other stages. Do we want to swap to *_sce.fcl like production is using?

cjbacchus commented 2 years ago
  1. Personally I'm not aware of anyone using anatree
  2. I wonder if the best approach is to inherit from that fcl and override to a small number of systs/universes, just so that we can check the infrastructure that propagates the weights works at least.
henrylay97 commented 2 years ago

I wonder if the best approach is to inherit from that fcl and override to a small number of systs/universes, just so that we can check the infrastructure that propagates the weights works at least.

Thanks Chris! As someone who's never used this functionality would you be able to suggest to me a good set of test values for these?

cjbacchus commented 2 years ago

Maybe @Ooohu can help you here. I think turning one or two of the most important dials by +/-1sigma sounds like enough

henrylay97 commented 2 years ago

Okay so I have a working set of tests. With regards to the conversation above I have gone back to using fcls with the weighting procedures but turned the number of universes down to 1. This tests the functionality but keeps it manageable as a regression test.

An example of these test can be found here and here. The former tests with respect to develop and therefore shows a good example of how differences will show up due to the initialisation issues addressed in SBNSoftware/sbnanaobj#33. The latter tests using the previous PR and therefore shows what a clean output would appear like.

I'll leave this open for feedback and then make a PR somepoint next week.

henrylay97 commented 2 years ago

PR opened SBNSoftware/sbndcode#233

wesketchum commented 2 years ago

Given the mentioned PRs have been merged in ... are these part of the tests in CI/validation now @henrylay97 @chilge ? If so, I'd propose closing this issue, and we can make new ones with better specificity as needed. If not, I'd propose we leave it open, and push to complete ASAP.

henrylay97 commented 2 years ago

These tests are fully integrated for SBND and have been running as standard since December. The functionality is there for ICARUS to use as well but I don't believe they have been setup to do so yet?