Remove dependence on weights file

[eacharles]

https://jira.slac.stanford.edu/projects/LK/issues/LK-153

[donhorner]

JIRA Description:

When binnedLikelihood is called with the wmap option (in order to apply weights), the weights file is apparently accessed and the call breaks when the file is moved.
As far as I understand how it works, all the information is stored in the weights extension, so the file is not necessary.
Would there be any drawback to removing the access to the weights file when the weights extension exists?
The wmap argument would then be there only as a boolean (yes/no).

[jasercion]

@dagorym was this resolved in any of the work you did on Jean's requests? It seems like it may be related.

[dagorym]

No, this is not something I looked at.

[dagorym]

I don't have access to JIRA anymore. Were there any test cases with this?

The C++ class never directly reads the file, but is passed and object with the file data already read, and the first check it does it to see if the weights extension exists and uses that, ignoring the passed in map. It might be useful to set the m_weightMap_orig parameter to NULL in that case (which is currently not being done) since it is never used. But I don't think that is really an issue in this case.

Looking upstream, the likelihood tool itself does attempt to read in the weight map file and does not check to see if it is needed or not (The same is true in the Python BinnedAnalysis class). So if a file name is provided that doesn't exist, it will fail. But if it does exist, it will be loaded into memory and then what happens to the file is irrelevant as it is cached and not read again at any time.

It looks like the way the code is written, this is what happens:

1. If no weights file name is provided, the tool assumes that weights are not to be applied
2. If weights are wanted, the user supplies a filename of a weights map to indicate that weights should be applied
3. The weights map is loaded and passed to the Likelihood object
4. If the source file already has a weights extension, the passed in object is ignored and the weightMap is created from the data in the extension and stored in the data cache. Note that a reference to the passed-in map read from the weights file is stored in the BinnedCountsCache object (this is what I said should probably be set to NULL above)
5. If there is no weights extension, a weightMap is generated from the passed in map and stored in the data cache.

If the weights extension exists, there is nothing that requires the filename provided for the weight map to actually be an actual weight map file or even the correct one, other than it has to be a map file that can be read in. It is never used. This is probably where the JIRA issue originated. They processed the weight map file to make the weights extension and deleted it but still had to give the filename to trigger using the weights and it failed because the attempt to read the nonexistent file fails.

If it is true that all source map files that need weighting have the weights extension, then we could change the parameter into a boolean to indicate using the weights or not. This would be a major interface change on the Python side and could potentially break any existing scripts. Not 100% sure how we document that and convey that to the users. I think I could maybe make it a non-issue for old scripts but I'd need a few test cases. Again, this only works if 100% of all source map files that are to be weighted have the weights extension.

However, if it is possible to pass in a source map file that doesn't have a weights extension and we need to also pass in a weight map filename to be loaded, we either have to keep things as is, or keep the wmap parameter as the filename (current functionality) and add a second parameter to be the boolean flag for whether or not to apply weighting. If that parameter is true, check for the weights extension and use it if it does, otherwise load the file passed in as the wmap parameter (which could be left empty if the weights extension exists.)

If we want to change the parameters (either the type or adding a new one), we need to look though all the code to find all the references and make sure they work properly.

Either is doable, we just need to decide which we want to do.

[jasercion]

Marking as Deferred for now, since we are unsure if the weights files are something that are still used/applicable by Fermitools users. @dagorym will reach out to Jean to see if this is still a necessary change/ask for additional use case context. Will move this back to active development if needed.

[phbruel]

Regarding the use of the likelihood weights, I can certainly answer that they are definitely used by FermiTools users.

[jballet]

I fear there was a misunderstanding when I wrote that the wmap argument could be there as a boolean. I did not mean that it should be turned into a boolean, because of course it needs to be a file name on the first call, when the weights extension does not exist yet.
What I meant is that it should be used as a boolean, namely as a trigger that the user actually wants to apply weights, but the file should not be opened, let alone stored to use memory for no purpose, unless it is actually needed (ie, unless the weights extension does not exist already).

[dagorym]

Okay, It is used as a boolean in the sense Jean describes here in the C++ code where the weight map is actually first checked and instantiated. However, the hire level code, both in the command-line gtlike and Python BinnedAnalysis class just load the map file and pass the resulting object on to the weight map cache where the boolean check occurs. And those are just the two places I found. I don't know if there are others (I haven't looked yet) that invoke this that would also have to be modified.

So this is possible, but would take a bit of restructuring the code in several different places and possibly changing the interface to the weight map cache object to make it happen.

[jballet]

This is related to issue #127 aiming at reducing the memory used by those weight files.