Skip to content

Commit

Permalink
Add a use-all-timesteps flag.
Browse files Browse the repository at this point in the history
By default, hyperdrive di-calibrate and solutions-apply only use
unflagged data. To also use flagged data, one had to see what timesteps
were flagged, stop the process, then provide all timesteps. This new
flag prevents that legwork.

Also add the ability to select timesteps in vis-subtract; it didn't
allow users to control timesteps at all previously. No use-all-timesteps
flag is needed because vis-subtract uses all data by default.
  • Loading branch information
cjordan committed Aug 23, 2022
1 parent 0bf83c8 commit a8d2e83
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 10 deletions.
7 changes: 7 additions & 0 deletions src/calibrate/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,11 @@ pub struct CalibrateUserArgs {
#[clap(long, multiple_values(true), help_heading = "CALIBRATION")]
pub timesteps: Option<Vec<usize>>,

/// Use all timesteps in the data, including flagged ones. The default is to
/// use all unflagged timesteps.
#[clap(long, conflicts_with("timesteps"), help_heading = "CALIBRATION")]
pub use_all_timesteps: bool,

#[clap(long, help = UVW_MIN_HELP.as_str(), help_heading = "CALIBRATION")]
pub uvw_min: Option<String>,

Expand Down Expand Up @@ -362,6 +367,7 @@ impl CalibrateUserArgs {
timesteps_per_timeblock,
freq_average_factor,
timesteps,
use_all_timesteps,
uvw_min,
uvw_max,
max_iterations,
Expand Down Expand Up @@ -405,6 +411,7 @@ impl CalibrateUserArgs {
timesteps_per_timeblock: cli_args.timesteps_per_timeblock.or(timesteps_per_timeblock),
freq_average_factor: cli_args.freq_average_factor.or(freq_average_factor),
timesteps: cli_args.timesteps.or(timesteps),
use_all_timesteps: cli_args.use_all_timesteps || use_all_timesteps,
uvw_min: cli_args.uvw_min.or(uvw_min),
uvw_max: cli_args.uvw_max.or(uvw_max),
max_iterations: cli_args.max_iterations.or(max_iterations),
Expand Down
8 changes: 5 additions & 3 deletions src/calibrate/params/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ impl CalibrateParams {
timesteps_per_timeblock,
freq_average_factor,
timesteps,
use_all_timesteps,
uvw_min,
uvw_max,
max_iterations,
Expand Down Expand Up @@ -433,10 +434,11 @@ impl CalibrateParams {
};

let timesteps_to_use = {
match timesteps {
None => Vec1::try_from_vec(obs_context.unflagged_timesteps.clone())
match (use_all_timesteps, timesteps) {
(true, _) => obs_context.all_timesteps.clone(),
(false, None) => Vec1::try_from_vec(obs_context.unflagged_timesteps.clone())
.map_err(|_| InvalidArgsError::NoTimesteps)?,
Some(mut ts) => {
(false, Some(mut ts)) => {
// Make sure there are no duplicates.
let timesteps_hashset: HashSet<&usize> = ts.iter().collect();
if timesteps_hashset.len() != ts.len() {
Expand Down
3 changes: 3 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,9 @@ impl From<VisSubtractError> for HyperdriveError {
| VisSubtractError::NoSourcesAfterVeto
| VisSubtractError::NoSources
| VisSubtractError::AllSourcesFiltered
| VisSubtractError::NoTimesteps
| VisSubtractError::DuplicateTimesteps
| VisSubtractError::UnavailableTimestep { .. }
| VisSubtractError::NoInputData
| VisSubtractError::InvalidDataInput(_)
| VisSubtractError::BadArrayPosition { .. }
Expand Down
13 changes: 10 additions & 3 deletions src/solutions/apply/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ pub struct SolutionsApplyArgs {
#[clap(long, multiple_values(true), help_heading = "INPUT FILES")]
timesteps: Option<Vec<usize>>,

/// Use all timesteps in the data, including flagged ones. The default is to
/// use all unflagged timesteps.
#[clap(long, conflicts_with("timesteps"), help_heading = "INPUT FILES")]
use_all_timesteps: bool,

#[clap(
long, help = ARRAY_POSITION_HELP.as_str(), help_heading = "INPUT FILES",
number_of_values = 3,
Expand Down Expand Up @@ -168,6 +173,7 @@ fn apply_solutions(args: SolutionsApplyArgs, dry_run: bool) -> Result<(), Soluti
data,
solutions,
timesteps,
use_all_timesteps,
array_position,
tile_flags,
ignore_input_data_tile_flags,
Expand Down Expand Up @@ -463,9 +469,10 @@ fn apply_solutions(args: SolutionsApplyArgs, dry_run: bool) -> Result<(), Soluti

let tile_to_unflagged_cross_baseline_map =
TileBaselineMaps::new(total_num_tiles, &tile_flags).tile_to_unflagged_cross_baseline_map;
let timesteps = match timesteps {
None => Vec1::try_from(obs_context.unflagged_timesteps.as_slice()),
Some(mut ts) => {
let timesteps = match (use_all_timesteps, timesteps) {
(true, _) => Vec1::try_from(obs_context.all_timesteps.as_slice()),
(false, None) => Vec1::try_from(obs_context.unflagged_timesteps.as_slice()),
(false, Some(mut ts)) => {
// Make sure there are no duplicates.
let timesteps_hashset: HashSet<&usize> = ts.iter().collect();
if timesteps_hashset.len() != ts.len() {
Expand Down
9 changes: 9 additions & 0 deletions src/vis_utils/subtract/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ pub(crate) enum VisSubtractError {
)]
InvalidDataInput(&'static str),

#[error("The data either contains no timesteps or no timesteps are being used")]
NoTimesteps,

#[error("Duplicate timesteps were specified; this is invalid")]
DuplicateTimesteps,

#[error("Timestep {got} was specified but it isn't available; the last timestep is {last}")]
UnavailableTimestep { got: usize, last: usize },

#[error(
"An invalid output format was specified ({0}). Supported:\n{}",
*VIS_OUTPUT_EXTENSIONS,
Expand Down
38 changes: 34 additions & 4 deletions src/vis_utils/subtract/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ pub struct VisSubtractArgs {
#[clap(long, help = SOURCE_LIST_TYPE_HELP.as_str(), help_heading = "INPUT FILES")]
source_list_type: Option<String>,

/// The timesteps to use from the input data. The default is to use all
/// timesteps, including flagged ones.
#[clap(long, multiple_values(true), help_heading = "INPUT FILES")]
timesteps: Option<Vec<usize>>,

#[clap(
short = 'o',
long,
Expand Down Expand Up @@ -192,6 +197,7 @@ fn vis_subtract(args: VisSubtractArgs, dry_run: bool) -> Result<(), VisSubtractE
data,
source_list,
source_list_type,
timesteps,
outputs,
time_average,
freq_average,
Expand Down Expand Up @@ -431,7 +437,31 @@ fn vis_subtract(args: VisSubtractArgs, dry_run: bool) -> Result<(), VisSubtractE
}
};

let timesteps = &obs_context.all_timesteps;
let timesteps = match timesteps {
None => Vec1::try_from(obs_context.all_timesteps.as_slice()),
Some(mut ts) => {
// Make sure there are no duplicates.
let timesteps_hashset: HashSet<&usize> = ts.iter().collect();
if timesteps_hashset.len() != ts.len() {
return Err(VisSubtractError::DuplicateTimesteps);
}

// Ensure that all specified timesteps are actually available.
for &t in &ts {
if obs_context.timestamps.get(t).is_none() {
return Err(VisSubtractError::UnavailableTimestep {
got: t,
last: obs_context.timestamps.len() - 1,
});
}
}

ts.sort_unstable();
Vec1::try_from_vec(ts)
}
}
.map_err(|_| VisSubtractError::NoTimesteps)?;

let precession_info = precess_time(
obs_context.phase_centre,
obs_context.timestamps[*timesteps.first()],
Expand Down Expand Up @@ -638,7 +668,7 @@ fn vis_subtract(args: VisSubtractArgs, dry_run: bool) -> Result<(), VisSubtractE
}

let timeblocks =
timesteps_to_timeblocks(&obs_context.timestamps, time_average_factor, timesteps);
timesteps_to_timeblocks(&obs_context.timestamps, time_average_factor, &timesteps);

if dry_run {
info!("Dry run -- exiting now.");
Expand Down Expand Up @@ -712,7 +742,7 @@ fn vis_subtract(args: VisSubtractArgs, dry_run: bool) -> Result<(), VisSubtractE
obs_context,
&maps.tile_to_unflagged_cross_baseline_map,
input_data.deref(),
timesteps,
&timesteps,
vis_shape,
tx_model,
&error,
Expand Down Expand Up @@ -769,7 +799,7 @@ fn vis_subtract(args: VisSubtractArgs, dry_run: bool) -> Result<(), VisSubtractE
&obs_context.tile_names,
obs_context.obsid,
&obs_context.timestamps,
timesteps,
&timesteps,
&timeblocks,
time_res,
freq_res,
Expand Down

0 comments on commit a8d2e83

Please sign in to comment.