Freeze source parameters in FluxEstimator

[registerrier]

Description

This pull request modifies the behaviour of FluxEstimator freezing all the source of interest parameters to only leave the norm of the ScaleSpectralModel free.
This was discussed in #2877 but we never implemented it. It does not seem meaningful to leave the source spatial parameters free. Can we guarantee that it does not fit a different source?

Because the Fit and the Estimator classes are stateless, it does not seem possible to properly test that the source spatial parameters do not change during the various steps. Could there be a way to test this?

Dear reviewer
This changes the behavior rather strongly, but I can't really see a good reason to leave the non spectral parameters of the source of interest free. Opinions?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d4024ac) 75.77% compared to head (9010be3) 75.77%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4567   +/-   ##
=======================================
  Coverage   75.77%   75.77%           
=======================================
  Files         227      227           
  Lines       33310    33311    +1     
=======================================
+ Hits        25240    25241    +1     
  Misses       8070     8070           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[AtreyeeS]

This is the issue reported in #4472
Specially for upper limit computation, this becomes a big problem as the extension continuously keeps changing, so each upper limit corresponds to a different region. And, the fit sometimes goes ends on a different source.

@adonath suggested we leave it to the user to freeze all spatial parameters before running the estimator.
Then we should explain the behaviour and the caveats clearly.

[bkhelifi]

The suggestion of @adonath is wise, indeed. However, are the non-expert aware of the issue? Is there properly documented?
The other possibility is to add a comment or a note in the doctring...

[QRemy]

I vote to merge this.

[bkhelifi]

For me, this behaviour seems very reasonable and logical. So, I do approuve this PR.

One should however precise this behaviour in the docstring (and maybe in global rst page), and precising also that a global fit is needed as prior

[AtreyeeS]

I see no good reason to not merge this, so +1 from my side as well.
But this should be added in the estimator user guide, and the change in behaviour specifically mentioned in the release notes.

[QRemy]

Thanks for the feedback @AtreyeeS , @bkhelifi.
I modified the docs as suggested, could you have one last look before merging ?

[bkhelifi]

Thanks a lot @QRemy

[AtreyeeS]

Thank you @QRemy ! Merging this