From 78bd15900e178667a6496f096b45a2c7595e81b2 Mon Sep 17 00:00:00 2001 From: Dustin Jenkins Date: Wed, 14 Aug 2024 13:28:29 -0700 Subject: [PATCH] Code review rework and adjust CDELT values when present. --- .../opencadc/fits/slice/WCSCutoutUtil.java | 30 +++++++++--------- .../test/java/org/opencadc/fits/FitsTest.java | 5 ++- .../fits/slice/NDimensionalSlicerTest.java | 31 +++++++++++++++++++ 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/cadc-data-ops-fits/src/main/java/org/opencadc/fits/slice/WCSCutoutUtil.java b/cadc-data-ops-fits/src/main/java/org/opencadc/fits/slice/WCSCutoutUtil.java index 6320ae05..f25c4bf3 100644 --- a/cadc-data-ops-fits/src/main/java/org/opencadc/fits/slice/WCSCutoutUtil.java +++ b/cadc-data-ops-fits/src/main/java/org/opencadc/fits/slice/WCSCutoutUtil.java @@ -150,16 +150,15 @@ public static PixelRange[] getBounds(final Header header, final Cutout cutout) static void adjustHeaders(final Header header, final int dimensionLength, final int[] corners, final int[] steps) { // CRPIX values are not set automatically. Adjust them here, if present. for (int i = 0; i < dimensionLength; i++) { - final HeaderCard crPixCard = header.findCard(Standard.CRPIXn.n(i + 1)); if (crPixCard != null) { // Need to run backwards (reverse order) to match the dimensions. final double nextValue = corners[corners.length - i - 1]; - final int stepValue = steps[corners.length - i - 1]; - final double crPixValue = (Double.parseDouble(crPixCard.getValue()) - nextValue) / stepValue; + final int stepValue = steps[steps.length - i - 1]; + final double crPixValue = Double.parseDouble(crPixCard.getValue()) - nextValue; if (stepValue > 1) { - final double newValue = crPixValue + (1.0 - (1.0 / stepValue)); + final double newValue = (crPixValue / stepValue) + (1.0 - (1.0 / stepValue)); crPixCard.setValue(newValue); LOGGER.debug("Adjusted " + crPixCard.getKey() + " to " + newValue); } else { @@ -168,19 +167,20 @@ static void adjustHeaders(final Header header, final int dimensionLength, final } } - // Handle PC values. These typically override CD values, but as this is operating on Archive data, we'll simply - // modify the values as-is, meaning the CD values will be left even if a PC matrix is included. - // - // TODO: Does CDELTn need to come into play somewhere? - // jenkinsd 2024.08.13 + // CDELTn cards are typically present for PC matrices, but due to the nature of Archive data, + // they could be present even if a CD matrix is present. Since PC matrices are the default in + // the absence of a CD matrix, it's not unusual for it to be absent. + // See https://www.aanda.org/articles/aa/pdf/2002/45/aah3859.pdf // - for (int j = 0; j < dimensionLength; j++) { - final HeaderCard pcMatrixCard = header.findCard(String.format("PC%d_%d", i + 1, j + 1)); - if (pcMatrixCard != null) { - final double pcMatrixValue = Double.parseDouble(pcMatrixCard.getValue()); - pcMatrixCard.setValue(pcMatrixValue * (double) steps[i]); - } + final HeaderCard cDeltCard = header.findCard(Standard.CDELTn.n(i + 1)); + if (cDeltCard != null) { + final double cDeltValue = Double.parseDouble(cDeltCard.getValue()); + final int stepValue = steps[steps.length - i - 1]; + cDeltCard.setValue(cDeltValue * (double) stepValue); + } + // Handle the entire CD matrix. + for (int j = 0; j < dimensionLength; j++) { final HeaderCard cdMatrixCard = header.findCard(String.format("CD%d_%d", i + 1, j + 1)); if (cdMatrixCard != null) { final double cdMatrixValue = Double.parseDouble(cdMatrixCard.getValue()); diff --git a/cadc-data-ops-fits/src/test/java/org/opencadc/fits/FitsTest.java b/cadc-data-ops-fits/src/test/java/org/opencadc/fits/FitsTest.java index 3c546e23..a857235e 100644 --- a/cadc-data-ops-fits/src/test/java/org/opencadc/fits/FitsTest.java +++ b/cadc-data-ops-fits/src/test/java/org/opencadc/fits/FitsTest.java @@ -77,7 +77,6 @@ import nom.tam.fits.HeaderCard; import nom.tam.fits.header.IFitsHeader; import nom.tam.fits.header.Standard; -import nom.tam.fits.header.WCS; import nom.tam.fits.header.extra.NOAOExt; import org.apache.log4j.Level; import org.apache.log4j.Logger; @@ -93,10 +92,10 @@ public class FitsTest { private static final IFitsHeader[] HEADER_CARD_KEYS_TO_CHECK = new IFitsHeader[]{ Standard.BITPIX, Standard.NAXIS, Standard.EXTNAME, Standard.XTENSION, Standard.SIMPLE, Standard.EXTVER, - Standard.BSCALE, Standard.BUNIT, NOAOExt.CD1_1 + Standard.BSCALE, Standard.BUNIT, NOAOExt.CD1_1, Standard.CDELTn.n(1), Standard.CRPIXn.n(1) }; - public static void assertFitsEqual(final Fits expected, final Fits result) throws Exception { + public static void assertFitsEqual(final Fits expected, final Fits result) { final BasicHDU[] expectedHDUList = expected.read(); final BasicHDU[] resultHDUList = result.read(); diff --git a/cadc-data-ops-fits/src/test/java/org/opencadc/fits/slice/NDimensionalSlicerTest.java b/cadc-data-ops-fits/src/test/java/org/opencadc/fits/slice/NDimensionalSlicerTest.java index e7bb8a35..e090c6b6 100644 --- a/cadc-data-ops-fits/src/test/java/org/opencadc/fits/slice/NDimensionalSlicerTest.java +++ b/cadc-data-ops-fits/src/test/java/org/opencadc/fits/slice/NDimensionalSlicerTest.java @@ -85,6 +85,7 @@ import org.apache.log4j.Level; import org.apache.log4j.Logger; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; import org.opencadc.fits.FitsTest; import org.opencadc.fits.NoOverlapException; @@ -101,6 +102,36 @@ public class NDimensionalSlicerTest { Log4jInit.setLevel("org.opencadc.fits", Level.DEBUG); } + @Test + @Ignore("Requires larger ALMA file. Useful for running locally.") + public void testIncorrectWCS() throws Exception { + ExtensionSliceFormat fmt = new ExtensionSliceFormat(); + List slices = new ArrayList<>(); + slices.add(fmt.parse("[*:4,*:5,*:4]")); + final Cutout cutout = new Cutout(); + cutout.pixelCutouts = slices; + + final NDimensionalSlicer slicer = new NDimensionalSlicer(); + final File file = FileUtil.getFileFromResource("test-alma-cube.fits", NDimensionalSlicerTest.class); + + final String configuredTestWriteDir = System.getenv("TEST_WRITE_DIR"); + final String testWriteDir = configuredTestWriteDir == null ? "/tmp" : configuredTestWriteDir; + final File expectedFile = FileUtil.getFileFromResource("test-alma-cube-cutout.fits", + NDimensionalSlicerTest.class); + final Path outputPath = Files.createTempFile(new File(testWriteDir).toPath(), "test-alma-cube-cutout", ".fits"); + LOGGER.debug("Writing out to " + outputPath); + + try (final OutputStream outputStream = Files.newOutputStream(outputPath.toFile().toPath())) { + slicer.slice(file, cutout, outputStream); + } + + final Fits expectedFits = new Fits(expectedFile); + final Fits resultFits = new Fits(outputPath.toFile()); + + FitsTest.assertFitsEqual(expectedFits, resultFits); + Files.deleteIfExists(outputPath); + } + @Test public void testMEFFileSlice() throws Exception { ExtensionSliceFormat fmt = new ExtensionSliceFormat();