Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reducing skymodel confusion #329

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Reducing skymodel confusion #329

wants to merge 3 commits into from

Conversation

nicholebarry
Copy link
Contributor

Some skymodel behaviour was very opaque, as pointed out by Shijiao. So, I set out to make it a little more obvious what is going on.

Previous functionality
When return_cal_visibilities is set, then the calibration visibilities are kept around after calibration. If a different set of visibilities need to be calculated for the subtraction model, only sources that are extra to the initial calibration sources list are calculated and added to the returned calibration visibilities. This assumes that the calibration sources are a subset of the model sources. While this reduces the number of calculations, it is not obvious from the user that setting return_cal_visiblities would cause this behaviour.

The skymodel that is saved, regardless of whether or not return_cal_visibilities is set, only includes model sources that are not "duplicates" of the calibration sources. There is no indication in the source list whether a source was used in the model. Assumptions include 1) that the model is a subset, and 2) that sources that are co-located to within 1/100th of a pixel are duplicates, regardless of brightness.

New functionality
When return_cal_visibilities is set, then the calibration visibilities are kept around after calibration. If a different set of visibilities need to be calculated for the subtraction model, all will be calculated unless combine_skymodels is set. This gives flexibility to do the previous functionality (and be more efficient in your number of calculations) or do something more complicated where the assumptions do not apply.

When combine_skymodels is set, the code attempts to combine the skymodels as above. Extra model sources are marked with unsetting the include_calibration keyword (an already present keyword) for that source in the skymodel; duplicates are not included. If it seems as though the model is not a subset of calibration, then all sources are saved.

If combine_skymodels is not set, then code will save separate skymodels.

save_skymodels must be set for skymodels to be saved.

Copy link
Member

@bhazelton bhazelton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a really good improvement, much more predictable behavior. one question I have is how does this interact with the current "standard" run? I feel like it doesn't -- the standard run doesn't use a different set of sources for calibration vs subtraction. Mostly wondering who all needs to be informed of this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants