caracal-pipeline / caracal

Containerized Automated Radio Astronomy Calibration (CARACal) pipeline
GNU General Public License v2.0
28 stars 6 forks source link

issue with repeated model subtraction #1544

Closed paoloserra closed 7 months ago

paoloserra commented 9 months ago

@dane-kleiner I think I have found a small issue with the model subtraction in the line worker. Consider the following situation:

I know I can force model subtraction (so I am not stuck), but this should not be necessary.

I think that the solution would be that whenever we write to the CORRECTED column in the selfcal worker the number of model subtraction recorded in the MS is reset to zero.

Do you agree? And could you take care of this?

dane-kleiner commented 9 months ago

I see what you're saying but I'm not convinced that we should change the code. Firstly, it's functioning as designed, and the force option is there precisely for this usage. I mean, it's not a bug, it's a feature!

I'm less inclined to change anything as what you're doing is non-standard, so wouldn't it be better for you to keep track of how many times you've subtracted the model? Rather than resetting it because you're re-running selfcal multiple times?

I do see your point though and it shouldn't be a difficult implementation, but I'm not (yet) convinced that we should.

dane-kleiner commented 8 months ago

In your example above, regardless of wether you make a different model, don't you need to add the model back into the CORRECT data before subtracting the new model? If you do this, it should be fine, because adding a model will reset the counter to 0. This is what my 3am brain realised last night, but I may be missing something.

paoloserra commented 8 months ago

When you repeat selfcalibration you typically overwrite both MODEL and CORRECTED. The latter gets overwritten by DATA*GAIN, so all memory of previous MODEL subtraction/addition is lost. That's why the counter should be set to 0.

If you only overwrite MODEL (e.g., you re-image but do not re-calibrate based on the new MODEL) I agree that you would first need to add the old MODEL to CORRECTED in order for things to work out well. So the counter should not be set to 0. In my opinion it's up to the user to be careful with what they are doing in that situation.

I think that all this was already contained in my initial suggestion:

I think that the solution would be that whenever we write to the CORRECTED column in the selfcal worker the number of model subtraction recorded in the MS is reset to zero.

dane-kleiner commented 7 months ago

But in any situation that you've already done a model subtraction and you want to re-do the model subtraction, regardless if it's from a new model or from a new calibration, you first need to add the (original) model back into the MS. No?

paoloserra commented 7 months ago

I think that the key is whether you're re-doing selfcal or not. If you are, you're going to start again from DATA, and overwrite CORRECTED with DATA*GAIN. So it does not matter whether the CORRECTED has the old MODEL subtracted or not. You're overwriting it anyway. So why should you bother undoing the old MODEL subtraction?

dane-kleiner commented 7 months ago

Ok, just to summarise and have this in writing.

If transfer_apply_gains or calibrate = True, then check for counter and reset it if it's there. Otherwise, do nothing.