Autocorrclean

[hmuellergoe]

Major code in this request:
-> Implementation of autocorr-CLEAN

includes also a few, minor updates and some experimental code for other methods:

• option to work with odd number of pixels in Asp-CLEAN
• expose LBFGSB quantities (tolerances, exit criteria) to the top level
• expose scales for Asp-CLEAN to the top level
• add an option to never switch to Hogbom in Asp-CLEAN when fusedthreshold is set to a negative number
• experimental code for approximating the psf as a sum of wavelets, to accelerate Asp-CLEAN LBFGS computation (that is like the Zhang version, but instead of a simple Gaussian approximating the main lobe, we use a few Gaussians approximating the main lobe and the first few sidelobes), not robustly tested yet
• experimental alternative multifrequency Asp-CLEAN option: in which the Asp's are defined from a continuum image. Then the spectral imaging bypasses the iterative CLEAN part of Asp-CLEAN, simply reads in the continuum Asps, and for every spectral channel just solves the LBFGSB step (i.e. adapts the Asps locally to what is needed in a respective spectral channel), not robustly tested now
• added options to save and load helper products

[preshanth]

@hmuellergoe and @agawatw the building is failing on casacpp which means some of your changes have not propagated into this branch. Can you make sure the code compiles before we carry out a review please.

[preshanth]

@hmuellergoe Once you fix the humbee tests we should be in a place to begin a proper review. @agawatw please detail any comments you have here.

[preshanth]

@hmuellergoe the tests are still failing for both humbee and asp

[agawatw]

There should be better software architecture to implement the experimental features without touching the base AspClean class API for the additional parameters, like waveletXXX. The Asp code seems harder for maintenance now because of the new mfasp conditional loop.

Also, please address the following comments and the duplicated code for the fusedthreshold.

[agawatw]

@hmuellergoe These added parameters seem fine but why do we need to expose lbfgsbepsf/lbfgsbepsx/lbfgsbepsg/lbfgsmaxit (LBFGSB optimization parameters of Asp-CLEAN)? Is it for the purpose of CG-Clean or autocorrelation, or just for better Asp-Clean performance if users know how to fine tune them?

[agawatw]

Why does "scales" need to be provided to Asp-Clean since the advantage of Asp-Clean is that it can automatically determined the optimal scale without users' input? Is it for computational efficiency? I see you added a new function of "loadInitScaleXfrs(itsScales);", and is the "scales" for that purpose?

If it is added for efficiency, have you measured the performance improvement to justify the added parameter?

[agawatw]

Line 272 to 279 are duplicated functionality since this has already been implemented in AspClean. See the function switchedToHogbom(), where
if (itsFusedThreshold < 0) itsSwitchedToHogbom = false;
The tclean user guide has already mentioned that if fusedthrehold=-1, it never switches to hogbom.

[agawatw]

This warning message is wrong. See the other comment about the already existed functionality that fusedthreshold=-1 is valid which tells AspClean never switches to hogbom. In this if(itsFusedThreshold < 0) condition, when fusedthreshold=-1, it prints this warning message which is incorrect.

[agawatw]

&& itsstopMS here is a duplicated addition as well since there is already !itsSwitchedToHogbom in this if condition.

[agawatw]

If the added code in MatrixCleaner.cc is purely for debug information, we should not check it in. Or at least, not place them in MatrixCleaner.cc which is the base class of every deconvolver.

[agawatw]

I don't think when it has been "switched to hogbom" (i.e. using only delta scale), the optimal strength should be set to 0 (i.e. Optimums(ii) = itsStrengthOptimum;.
Also, if the purpose of having these 3 additional variables
casacore::Vector<casacore::Float> Optimums(itsMaxNiter); casacore::Vector<casacore::Float> ScaleSizes(itsMaxNiter); casacore::Vector<casacore::IPosition> Positions(itsMaxNiter);
is purely for debug information, it is not necessary since there are already variables in AspClean holding this information already.

[sanbw]

Perhaps it will be useful to describe the general design of the deconvolver code in LibRA to @Hendrik ***@***.***> to figure out how to re-factor his code and perhaps some similar re-factoring in the Asp code. @genie ***@***.***> is perhaps well position to describe this. I feel once we figure out on paper how best to deploy new code, it will also become clear what needs to be done. My feeling is that in the end, the required work will not be large. If I can help in this, please let me know and we can discuss it together as well.

________________________________ From: agawatw ***@***.***> Sent: Friday, March 13, 2026 12:59 PM To: ARDG-NRAO/LibRA ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [ARDG-NRAO/LibRA] Autocorrclean (PR #1) {External} @agawatw commented on this pull request.

________________________________ In src/synthesis/MeasurementEquations/AspMatrixCleaner.cc<#1 (comment)>:

@@ -341,6 +372,15 @@ Int AspMatrixCleaner::aspclean(Matrix<Float>& model,

vecItsStrengthOptimum.push_back(itsStrengthOptimum); vecItsOptimumScaleSize.push_back(itsOptimumScaleSize); } + + if(itsSwitchedToHogbom){ + Optimums(ii) = 0.0; I don't think when it has been "switched to hogbom" (i.e. using only delta scale), the optimal strength should be set to 0 (i.e. Optimums(ii) = itsStrengthOptimum;. Also, if the purpose of having these 3 additional variables casacore::Vector<casacore::Float> Optimums(itsMaxNiter); casacore::Vector<casacore::Float> ScaleSizes(itsMaxNiter); casacore::Vector<casacore::IPosition> Positions(itsMaxNiter); is purely for debug information, it is not necessary since there are already variables in AspClean holding this information already. — Reply to this email directly, view it on GitHub<#1 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADG5B3KKLHDP46GL4NBRMYT4QRLBTAVCNFSM6AAAAACVOUHS4OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTSNBWGMZTSNZVGI>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***> [ { ***@***.***": "http://schema.org", ***@***.***": "EmailMessage", "potentialAction": { ***@***.***": "ViewAction", "target": "#1 (review)", "url": "#1 (review)", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { ***@***.***": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

[hmuellergoe]

That would be probably helpful. However, I already refactored most of it today by implemeting the spectal and wavelet option simply as an inherited subclass of Asp-CLEAN, leaving the original Asp-CLEAN code untouched, looks most practical to me from a maintenance point of view.

[agawatw]

@hmuellergoe The recommended software architecture is as followed, which will eliminates all if(itsWaveletTrigger) checks and wavelet-specific parameters from the base implementation and facilitate future experimental work and better maintainability.

1. Keep the base algorithm (AspClean) stable while improving it for extensibility.
2. Move experimental behavior into a subclass (e.g. waveletAspClean)
3. Use interchangeable strategies for scale models and normalization.

I'll work on item 1 and 3 above which will have usage examples of how to create a new scale model and normalization method in the subclass that inherits from AspClean.

For example, in terms of Item (3), there will be a new ScaleModel:
class ScaleModel {
public:
virtual ~ScaleModel() {}
virtual float value(
int i, int j,
int refi, int refj,
double scaleSize) const = 0;
};

In Asp, the gaussian scale model initialization will be changed to:
class GaussianScaleModel : public ScaleModel {
public:
double value(
int i, int j,
int refi, int refj,
float scaleSize) const override
{
return (1.0/(sqrt(2*M_PI)*scaleSize)) *
exp(-(pow(i-refi,2)+pow(j-refj,2)) *
0.5 / pow(scaleSize,2));
}
};

with a new function to pass this to the existing Asp function that sets up the scales:
void setScaleModel(std::unique_ptr model)
{
itsScaleModel = std::move(model);
}

Then, in the existing Asp's makeScaleImage function, the code will be changed from
if(itsWaveletTrigger == false){
iscale(i,j) = (1.0/(sqrt(2*M_PI)*scaleSize))exp(-(pow(i-refi,2) + pow(j-refj,2))0.5/pow(scaleSize,2)); // gaussian
}
else{
iscale(i,j) = (1.0/(2M_PIpow(scaleSize,2)))*exp(-(pow(i-refi,2) + pow(j-refj,2))*0.5/pow(scaleSize,2)); // wavelet
}

to

iscale(i,j) =
itsScaleModel->value(i,j,refi,refj,scaleSize);

This is just an example change. You'd need to implement your own
class WaveletScaleModel : public ScaleModel {}.

Also, the new WaveletAspClean would be something like:
class WaveletAspClean : public AspClean {
private:
double itsLbfgsEpsF;
double itsLbfgsEpsX;
double itsLbfgsEpsG;
int itsLbfgsMaxit;
casacore::Vector itsWaveletScales;
casacore::Vector itsWaveletAmps;
public:
WaveletAspClean()
{
setScaleModel(std::make_unique < WaveletScaleModel > ());
}
};

So there will be no wavelet parameters exist in AspClean.

[preshanth]

@hmuellergoe the code upstream has been updated a lot. It is time to rebase main onto your code and then come back and push here with --force. Then @agawatw and @preshanth will approve the PR and then I will grab this code and commit history and make sure it comes in from gitlab.

[agawatw]

@hmuellergoe I wrote a README in synthesis/MeasurementEquations that describes how to derive from the AspClean base class with examples. Once you have updated your code accordingly, please check if you still get the same WaveletAsp and auto_CS results as before. We''ll then do the PR and I'll add unit tests for those in the ardg/main gitlab.