Skip to content

fix the bank splitter bug that group random subbanks. + modify the mu-sorting

Leo Tsukada requested to merge master-bank_splitter_fix into master

Summary

We recently discovered the current bank splitter accidentally bundle sub-banks coming from random range in \chi_{eff}. See the violin plot below (200 templates * 5 sub-banks).

image

I found that this line is re-sorting sub-banks based on the parameter set by options.sort_by regardless of \chi_{eff}. Rather, we want them to be sorted inside each \chi_{eff} bin.
So I made a change so that it groups multiple sub-banks into one svd bank in each \chi_{eff} and that svd bins are still sorted by options.sort_by afterwards, which gives us better sense of which svd id roughly corresponds to BNS/NSBH/BBH.
After this fix, the violin plot looks much better.
image

Caveats about incomplete split-banks

There is a potential caveat raised during the east call that there is at most one bin per chi-bin or mu2-bin that has incomplete split-banks (e.g. 1 split-bank when --num-banks=2). When counting the number of split banks for every bin, I found only 20 out of 1320 is incomplete (see the pie chart below). During the discussion in UWM F2F, Chad is convinced that will not be a big problem unless we set unpractically large --num-banks e.g. 10. To be safe, I also added an error message when --num-banks >3, which warns users to know what they know what they are doing. --numbanks-force can force the program to run through.
image

fref fix from 100Hz to 200Hz

I changed the fref to be 200Hz and ran the floplator (a program that computes the compression rate for each svd bin) on the new svd bank files. The rate actually got slightly worse by ~50%. so I can see that the change in fref is not as impactful as I thought. please find comparison plots below.

fref=100Hz
image image

fref=200Hz
image image

Edited by Leo Tsukada

Merge request reports