SBNSoftware / sbnci

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

Add in fhicl-checks as part of sbn ci? #1

Closed wesketchum closed 2 years ago

wesketchum commented 3 years ago

May be nice to have automated fhicl checks in place for SBN CI, to test against a set of desired values.

absolution1 commented 3 years ago

1000% yes. This can be looked at.

wesketchum commented 3 years ago

I wrote some things a bit back, which I used for some basic checks in uboone. I think they would likely work with little modification, and may be a good base. Info dump from an old email here (so I hope it's all up to date) ...

I don't think something like this (other than Joseph) exists, so I've written something very simple, that builds off of Herb's previous work on the fhicl python module. (Perhaps, Herb, we should think about how to add some of these if they seem useful?)

See here: ~wketchum/fcl_check

In that directory there are three files:

fhicl_tools.py : some simple definitions for doing checks. It's barely commented, but you're welcome to look and welcome to expand.

fcl_check.py: an example python script that uses this. I read in a reco2 fcl file, and then create a comparison dictionary like so:

my_comps={ 'BeamWindowStartTime':{'key':"physics.producers.pandora.SliceIdTool.BeamWindowStartTime", 'value':3.57, 'op':"=" }, 'BeamWindowEndTimeFAIL':{'key':"physics.producers.pandora.SliceIdTool.BeamWindowEndTime",'value':5.15,'op':"="} }

They dictionary keys are like test names, and each should have a "key" (parameter in a pset to check), "value" (the value to compare against), and an "op" operation ("=","<","<=", etc.)

That dictionary interface means it's also easy to write a comparison fcl file and read that in to do the comparisons. An example is in that directory (reco2_comp.fcl) and looks like this:

BeamWindowStartTime:{ key:"physics.producers.pandora.SliceIdTool.BeamWindowStartTime" op:"=" value:3.57 } BeamWindowEndTime:{ key:"physics.producers.pandora.SliceIdTool.BeamWindowEndTime" op:"=" value:5.25 } BeamWindowEndTimeFail:{ key:"physics.producers.pandora.SliceIdTool.BeamWindowEndTime" op:"=" value:5.15 } BeamWindowTimeComp:{ key:"physics.producers.pandora.SliceIdTool.BeamWindowStartTime" op:"eval.<" value:"physics.producers.pandora.SliceIdTool.BeamWindowEndTime" }

(Note the last one: the "value" is set to be another parameter in the pset. If the 'op' key starts with 'eval', the code will evaluate the value from the parameter set, and then perform the comparison.)

You can run/test this yourself by setting up uboonecode, and then simply loading in the fhicl_tools.py module.

wesketchum commented 3 years ago

Assigning this to @chilge for the moment, for your delegation or otherwise consideration. Something like this would be a very nice early thing to add...

wesketchum commented 3 years ago

@jzennamo claims to have "tools from MicroBooNE" too, so he should comment/help with this.

wesketchum commented 3 years ago

@chilge : is this something that we can try to add very soon / ready for Aug13 release? At least to first order?

jzennamo commented 3 years ago

@chilge if you wanted me to pass along the uboone tools let me know, I can point you to what we wrote

chilge commented 3 years ago

somehow I missed the first post for this back in April :-| . yes @jzennamo - if you could point me to the uboone tools that would be very helpful. I will get back to you @wesketchum about if this timeline is feasible.

jzennamo commented 3 years ago

https://cdcvs.fnal.gov/redmine/projects/ubutil/repository/entry/scripts/fcl_check.sh?utf8=%E2%9C%93&rev=UBOONE_SUITE_v08_00_00_56

jzennamo commented 3 years ago

@wesketchum is the real champ on this one, but if you have any questions feel free to bug me, we should reduce the number of fcls that are checked (in uboone we checked ALL the fcls)

wesketchum commented 3 years ago

I'm being optimistic and putting it on the fast timeline. We can do it!

henrylay97 commented 3 years ago

I've put some initial work into this last week. Not actually using anything from the uboone tools but just developing a very simple ci_test for a list of fcls. Essentially I've written a script which takes a list of fcls and works through them performing a couple of checks. First it uses lar --debug_config which checks that the fcl can be parsed by fully expanding it etc. Second it performs a fhicl dump and then does a diff of this wrt to a reference version of that dump.

This script works as a simple check (I'm sure in future we'll want to develop it further though). Currently the things holding it back are: a) I haven't developed an auto method for updating the refs, have chatted to @vitodb and have options which I can pursue later this week b) It all lives in feature branches including for lar_ci which makes triggering more complex But these are both essentially just stopping it being all properly automatic this week, doesn't mean it can't be used in a manual way!

I've linked an example of this ci_test on the testmode dashboard. The stdout has the log results. In this example all the fcls pass the parsing check and the final fcl does have a difference in some parameters. The list of fcls used was arbitrarily chosen by myself to roughly represent a standard production but of course a proper list can be compiled with input from physics groups and production.

I know this is somewhat different to the options discussed above but it should provide a basic functionality to track fcl files changes. Happy to trigger it manually on anything we want whilst I work on the automation.

wesketchum commented 3 years ago

Great to hear @henrylay97 !

Leaving this issue open while we wait for the above things to be resolved, but I think this is a great start to this, and looking forward to talking more about it.

wesketchum commented 3 years ago

Moving this to 2021C project, though I think we should follow things through and aim to get something faster.

wesketchum commented 2 years ago

@henrylay97 @chilge any updates on this/thinking this may get in for 2021C?

chilge commented 2 years ago

required features added to sbndcode PR #208 by @henrylay97

possibility of porting to icaruscode being investigated by @rennney

chilge commented 2 years ago

Nice work @henrylay97 getting this up and running for SBND!

@rennney has informed us that ICARUS already does a fhicl check as part of its unit tests.

In the future, we might revisit this point in order to have consistent tests for the two experiments.