> perhaps the following code

[dtnguyen2]

perhaps the following code

    def teardown(self):
        self._use_caching = False

in the file models/model.py should be changed to:

   def teardown(self):
       self._use_caching = True

also

I personally don't have a good feel for how the cache code and the setup/teardown methods are meant to interact. Previously I think the cache was only used for models with no free parameters, and then only within the context of a "setup/teardown" call - e.g. as done by a fit call. This is presumably why there is a unilateral "turn off cacheing" in the teardown method, in case it had been turned on by the setup method.

We've now changed things so that cacheing is expected to be on, so turning it off in teardown isn't very useful. However, I am not convinced that always turning it on is a great idea either. My current feeling would be that the teardown method shouldn't change the _use_caching attribute. In that case we may also want to remove the "turn on caching if there are no free parameters" logic in the setup method as well (since we expect the cache support to already be on), but I think this is getting to the point that we should be reviewing the cache code.

Originally posted by @DougBurke in #678 (comment)

[dtnguyen2]

While working on PR #728 (a fix to issue #717), I decided to take a closer look at the following section of the code

    def teardown(self):
        self._use_caching = False

First of all, the current code does not pass the runtime cache option in the fit() command properly. This can be seen with the following modifications:

1. Add a simple print of the _use_caching value in the sherpa/models/model.py file:

 (sherpa) [dtn@devel12 sherpa]$ git diff sherpa/models/model.py
diff --git a/sherpa/models/model.py b/sherpa/models/model.py
index 5e4c0bf4..f465554a 100644
--- a/sherpa/models/model.py
+++ b/sherpa/models/model.py
@@ -49,6 +49,7 @@ def modelCacher1d(func):
 
     def cache_model(cls, pars, xlo, *args, **kwargs):
         use_caching = cls._use_caching
+        print('from cache_model: use_caching =', use_caching)
         cache = cls._cache
         queue = cls._queue

2. Slightly modify the sherpa-test-data/sherpatest/simultaneous-fit.py file, i.e. adding a couple of print statements for the value of _use_caching for the pl1 and pl2 models one before and after the call to fit(1, 2); Moreover, the original call to script, fit(1, 2) has an added optional argument cache=False:

(sherpa) [dtn@devel12 sherpatest]$ diff simultaneous-fit.py tmp.py
15,16c15,17
< 
< fit(1, 2)
---
> print('before fit: pl1._use_caching =', pl1._use_caching, 'pl2._use_caching =', pl2._use_caching)
> fit(1, 2, cache=False)
> print('after fit: pl1._use_caching =', pl1._use_caching, 'pl2._use_caching =', pl2._use_caching)

With the two aforementioned changes, when running the modified simultaneous-fit.py code one gets:

(sherpa) [dtn@devel12 sherpatest]$ python tmp.py
...
pl1._use_caching = True pl2._use_caching = True
from cache_model: use_caching = True
...
from cache_model: use_caching = True
Datasets              = 1, 2
Method                = levmar
Statistic             = chi2gehrels
Initial fit statistic = 4.69279e+11
Final fit statistic   = 7.4429 at function evaluation 41
Data points           = 18
Degrees of freedom    = 14
Probability [Q-value] = 0.916288
Reduced statistic     = 0.531636
Change in statistic   = 4.69279e+11
   abs1.nH        0.89819      +/- 0.397769    
   pl1.gamma      1.64501      +/- 0.486058    
   pl1.ampl       2.28327e-05  +/- 1.48213e-05 
   pl2.ampl       2.44592e-05  +/- 1.54245e-05 
pl1._use_caching = False pl2._use_caching = False

Notice that the before the fit, _use_caching for the pl1 and pl2 are True but after the fit, they are set to False due to the setting in the teardown code. The interesting thing here is the teardown method does not get called if one calls calc_stat instead of fit. Moreover, use_caching still has the value of True for the two models despite the runtime option of fit(1, 2, cache=False).

By making the following changes, ie set the class member self.use_caching to the runtime cache option in the startup and modify the teardown to set self.use_caching to False:

(sherpa) [dtn@devel12 sherpa]$ git diff sherpa/models/model.py
diff --git a/sherpa/models/model.py b/sherpa/models/model.py
index 5e4c0bf4..e2ca4169 100644
--- a/sherpa/models/model.py
+++ b/sherpa/models/model.py
@@ -49,6 +49,7 @@ def modelCacher1d(func):
 
     def cache_model(cls, pars, xlo, *args, **kwargs):
         use_caching = cls._use_caching
+        print('from cache_model: use_caching =', use_caching)
         cache = cls._cache
         queue = cls._queue
 
@@ -543,6 +544,7 @@ class ArithmeticModel(Model):
     def startup(self, cache):
         self._queue = ['']
         self._cache = {}
+        self._use_caching = cache
         if int(self.cache) > 0:
             self._queue = [''] * int(self.cache)
             frozen = numpy.array([par.frozen for par in self.pars], dtype=bool)
@@ -550,7 +552,7 @@ class ArithmeticModel(Model):
                 self._use_caching = cache
 
     def teardown(self):
-        self._use_caching = False
+        self._use_caching = True

When the modified script is run one gets the expected results:

(sherpa) [dtn@devel12 sherpatest]$ python tmp.py
...
before fit: pl1._use_caching = True pl2._use_caching = True
from cache_model: use_caching = False
...
from cache_model: use_caching = False
Datasets              = 1, 2
Method                = levmar
Statistic             = chi2gehrels
Initial fit statistic = 4.69279e+11
Final fit statistic   = 7.4429 at function evaluation 41
Data points           = 18
Degrees of freedom    = 14
Probability [Q-value] = 0.916288
Reduced statistic     = 0.531636
Change in statistic   = 4.69279e+11
   abs1.nH        0.89819      +/- 0.397769    
   pl1.gamma      1.64501      +/- 0.486058    
   pl1.ampl       2.28327e-05  +/- 1.48213e-05 
   pl2.ampl       2.44592e-05  +/- 1.54245e-05 
after fit: pl1._use_caching = True pl2._use_caching = True

That is during the fit, use_caching is set to False and after the fit, _use_caching for the pl1 and pl2 models are set to True. However, I don't think it is necessary to change the teardown method because, if fit were to be called again the startup method will initialize to the default value of True.

I will issue a PR to fix this issue shortly (after I finalize the test).