LSSTDESC / TJPCov

TJPCov is a general covariance calculator interface to be used within LSST DESC
https://tjpcov.readthedocs.io
MIT License
11 stars 1 forks source link

investigate key error #70

Closed felipeaoli closed 10 months ago

felipeaoli commented 1 year ago

Apparently, there is an inconsistency in overwrite option

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/tmp/ipykernel_151214/490363471.py in <module>
      1 # Or you can get the sacc file with the total covariance:
----> 2 s = cc.create_sacc_cov()
      3 
      4 ix = np.concatenate([s.indices(data_type='cl_00'),
      5                      s.indices(data_type='cl_0e'),

~/programs/lsst/TJPCov/tjpcov/covariance_calculator.py in create_sacc_cov(self, output, save_terms)
    165             return
    166 
--> 167         s = self.io.create_sacc_cov(cov, output)
    168 
    169         if save_terms:

~/programs/lsst/TJPCov/tjpcov/covariance_io.py in create_sacc_cov(self, cov, output, overwrite)
     85 
     86         s = self.get_sacc_file().copy()
---> 87         s.add_covariance(cov, overwrite=True)
     88 
     89         if os.path.isfile(output) and (not overwrite):

TypeError: add_covariance() got an unexpected keyword argument 'overwrite'
felipeaoli commented 1 year ago

In line 87 of covariance_io.py, set overwrite=overwrite instead of overwrite=True (avoid unintentional overwriting)

carlosggarcia commented 1 year ago

The overwrite option is in sacc>=0.7. There are two different things happening in create_sacc_cov.

  1. One, the addition or update of the covariance matrix stored in the input sacc file, which should always happen since the function is returning the sacc file with the new covariance.
  2. The writing of the sacc file to the disk. Here is where the overwrite argument plays a role: if the filename does not exist or the user wants to overwrite the existing file, it uses the given filename; otherwise, if it exists but the user does not want to overwrite it, it append the date when it was computed.

I think the current implementation is correct.