Skip to content

Commit

Permalink
Bug fixes for reading RTS solutions.
Browse files Browse the repository at this point in the history
Also remove the capability of writing RTS solutions (this wasn't correct
anyway), and update the online documentation.

As writing out calibrations solutions no longer requires a metafits
file, the API has been simplified.
  • Loading branch information
cjordan committed Oct 18, 2022
1 parent dd5cd20 commit 2ef57f6
Show file tree
Hide file tree
Showing 15 changed files with 613 additions and 482 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ and this project adheres to [Semantic
Versioning](https://semver.org/spec/v2.0.0.html).

## [0.2.1] - Unreleased
### Fixed
- Bugs were fixed surrounding the reading of RTS solutions.

### Changed
- RTS solutions may no longer be written out.
- Use "lto = "thin"". This makes performance slightly better and decreases the
size of the binary.
- Internal crates have been "folded" in with the main code. This should
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ indoc = "1.0.0"
marlu = { version = "0.8.0", features = ["approx"] }
ndarray = { version = "0.15.4", features = ["approx-0_5"] }
serial_test = "0.9.0"
tar = "0.4.38"
tempfile = "3.3.0"

[build-dependencies]
Expand Down
28 changes: 27 additions & 1 deletion mdbook/src/defs/cal_sols_rts.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,30 @@

![](https://media.giphy.com/media/NsIwMll0rhfgpdQlzn/giphy.gif)

todo!()
This format is extremely complicated and therefore its usage is discouraged.
However, it is possible to convert `RTS` solutions to one of the other supported
formats; a metafits file is required, and the *directory* containing the
solutions (i.e. `DI_JonesMatrices` and `BandpassCalibration` files) is supplied:

~~~admonish example title="Converting `RTS` solutions to another format"
```shell
hyperdrive solutions-convert /path/to/rts/solutions/ rts-as-hyp-solutions.fits -m /path/to/obs.metafits
```
~~~

Once in another format, the solutions can also be plotted.

An example of RTS solutions can be found in the `test_files` directory (as a
`.tar.gz` file). The code to read the solutions attempts to unpack and clarify
the format, but it is messy.

~~~admonish warning title="Writing `RTS` solutions"
I (CHJ) spent a very long time trying to make the writing of `RTS` solutions
possible, but ultimately gave up. One particularly difficult detail here is that
the RTS solutions contain a beam response; this could be either the MWA analytic
or FEE beam. But its application to the solutions is not clear and difficult to
reverse-engineer.
If you dare, there is incomplete commented-out code within `hyperdrive` that
attempts to write out the solutions.
~~~
3 changes: 1 addition & 2 deletions src/cli/solutions/apply/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,8 +473,7 @@ pub(crate) fn get_1090008640_identity_solutions_file(tmp_dir: &Path) -> PathBuf
..Default::default()
};
let file = tmp_dir.join("sols.fits");
sols.write_solutions_from_ext::<&Path, &Path>(&file, None)
.unwrap();
sols.write_solutions_from_ext::<&Path>(&file).unwrap();
file
}

Expand Down
2 changes: 1 addition & 1 deletion src/cli/solutions/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ impl SolutionsConvertArgs {
pub fn run(self) -> Result<(), HyperdriveError> {
let sols =
CalibrationSolutions::read_solutions_from_ext(&self.input, self.metafits.as_ref())?;
sols.write_solutions_from_ext(&self.output, self.metafits.as_deref())?;
sols.write_solutions_from_ext(&self.output)?;

info!(
"Converted {} to {}",
Expand Down
4 changes: 1 addition & 3 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,7 @@ impl From<SolutionsWriteError> for HyperdriveError {
fn from(e: SolutionsWriteError) -> Self {
let s = e.to_string();
match e {
SolutionsWriteError::UnsupportedExt { .. }
| SolutionsWriteError::RtsMetafitsRequired
| SolutionsWriteError::Rts(_) => Self::Solutions(s),
SolutionsWriteError::UnsupportedExt { .. } => Self::Solutions(s),
SolutionsWriteError::Fits(_) | SolutionsWriteError::Fitsio(_) => Self::Cfitsio(s),
SolutionsWriteError::IO(_) => Self::Generic(s),
}
Expand Down
8 changes: 1 addition & 7 deletions src/solutions/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use marlu::mwalib;
use mwalib::{fitsio, FitsError};
use thiserror::Error;

use super::rts::{RtsReadSolsError, RtsWriteSolsError};
use super::rts::RtsReadSolsError;

#[derive(Error, Debug)]
pub(crate) enum SolutionsReadError {
Expand Down Expand Up @@ -62,12 +62,6 @@ pub(crate) enum SolutionsWriteError {
#[error("Tried to write calibration solutions file with an unsupported extension '{ext}'!")]
UnsupportedExt { ext: String },

#[error("When interfacing with RTS calibration solutions, a metafits file is required")]
RtsMetafitsRequired,

#[error(transparent)]
Rts(#[from] RtsWriteSolsError),

#[error(transparent)]
Fitsio(#[from] fitsio::errors::Error),

Expand Down
35 changes: 10 additions & 25 deletions src/solutions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,37 +179,22 @@ impl CalibrationSolutions {
///
/// It is generally preferable to use [hyperdrive::write] for
/// hyperdrive-style files, because that allows more metadata to be written.
pub fn write_solutions_from_ext<P: AsRef<Path>, P2: AsRef<Path>>(
&self,
file: P,
metafits: Option<P2>,
) -> Result<(), HyperdriveError> {
Self::write_solutions_from_ext_inner(
self,
file.as_ref(),
metafits.as_ref().map(|f| f.as_ref()),
)
.map_err(HyperdriveError::from)
pub fn write_solutions_from_ext<P: AsRef<Path>>(&self, file: P) -> Result<(), HyperdriveError> {
Self::write_solutions_from_ext_inner(self, file.as_ref()).map_err(HyperdriveError::from)
}

pub(crate) fn write_solutions_from_ext_inner(
sols: &CalibrationSolutions,
file: &Path,
metafits: Option<&Path>,
) -> Result<(), SolutionsWriteError> {
if file.is_dir() {
let metafits = metafits.ok_or(SolutionsWriteError::RtsMetafitsRequired)?;
rts::write(sols, file, metafits)?;
} else {
let ext = file.extension().and_then(|e| e.to_str());
match ext.and_then(|s| CalSolutionType::from_str(s).ok()) {
Some(CalSolutionType::Fits) => hyperdrive::write(sols, file),
Some(CalSolutionType::Bin) => ao::write(sols, file),
None => Err(SolutionsWriteError::UnsupportedExt {
ext: ext.unwrap_or("<no extension>").to_string(),
}),
}?;
}
let ext = file.extension().and_then(|e| e.to_str());
match ext.and_then(|s| CalSolutionType::from_str(s).ok()) {
Some(CalSolutionType::Fits) => hyperdrive::write(sols, file),
Some(CalSolutionType::Bin) => ao::write(sols, file),
None => Err(SolutionsWriteError::UnsupportedExt {
ext: ext.unwrap_or("<no extension>").to_string(),
}),
}?;

Ok(())
}
Expand Down
Loading

0 comments on commit 2ef57f6

Please sign in to comment.