thm2isis now uses out attrs and converted to func #4504

Kelvinrr · 2021-06-02T17:14:34Z

Description

thm2isis now applies output attributes and converted to a func. Still needs gtests.

Related Issue

#4213

Motivation and Context

Before this bug fix, thm2isis would no honor output cube attributes

How Has This Been Tested?

original tests passing.

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Documentation change (update to the documentation; no code change)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have read and agree to abide by the Code of Conduct
I have read the CONTRIBUTING document.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.
I have added myself to the .zenodo.json document.
I have added any user impacting changes to the CHANGELOG.md document.

Licensing

This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words:

This work is free and unencumbered software released into the public domain. In jurisdictions that recognize copyright laws, the author or authors of this software dedicate any and all copyright interest in the software to the public domain.

I dedicate any and all copyright interest in this software to the public domain. I make this dedication for the benefit of the public at large and to the detriment of my heirs and successors. I intend this dedication to be an overt act of relinquishment in perpetuity of all present and future rights to this software under copyright law.

Kelvinrr · 2021-06-02T17:15:39Z

isis/src/odyssey/apps/thm2isis/thm2isis.cpp

+    CubeAttributeOutput outAttr = ui.GetOutputAttribute("to");
+
+    if((QString)inst["InstrumentId"] == "THEMIS_VIS") {
+      Cube *even = new Cube();
+      Cube *odd = new Cube();
+
+      even->setDimensions(p.Samples(), p.Lines(), p.Bands());
+      odd->setDimensions(p.Samples(), p.Lines(), p.Bands());
+
+      // even->setPixelType(Isis::Real);
+      // odd->setPixelType(Isis::Real);
+
+      QString evenFile = outFile.path() + "/" + outFile.baseName() + ".even.cub";
+      QString oddFile = outFile.path() + "/" + outFile.baseName() + ".odd.cub";
+
+      even->create(evenFile, outAttr);
+      odd->create(oddFile, outAttr);
+
+      frameletLines = 192 / ((int)inst["SpatialSumming"]);
+
+      outputCubes.push_back(odd);
+      outputCubes.push_back(even);
+    }
+    else {
+      Cube *outCube = new Cube();
+      outCube->setDimensions(p.Samples(), p.Lines(), p.Bands());
+
+      // outCube->setPixelType(Isis::Real);
+
+      outCube->create(outFile.expanded(), outAttr);


This is basically the bug fix (hard to find when you also do an app convert). Output attributes passed into Cube::create.

acpaquette

Overall this looks good! Did you try to replicate the test that I mentioned in the issue?

Kelvinrr · 2021-06-02T17:28:38Z

@acpaquette yeah, I ran the images in the test folders with different pixel types locally and they all worked as expected.

Kelvinrr · 2021-06-02T20:06:27Z

From writing the GTests for the output attributes. Something about making the pixel type to anything other than Real causes the pixel values to go to garbage.

hist output from a Themis image ingested with +16bit using the original test image

Cube:              output/thm2isis-Truth3.even.cub
Band:              1
Average:           N/A
Std Deviation:     N/A
Variance:          N/A
Median:            N/A
Mode:              N/A
Skew:              N/A
Minimum:           N/A
Maximum:           N/A

Total Pixels:      589824
Valid Pixels:      0
Pixels Below Min:  0
Pixels Above Max:  0
Null Pixels:       401724
Lis Pixels:        0
Lrs Pixels:        0
His Pixels:        0
Hrs Pixels:        188100
MinInclusive,MaxExclusive,Pixels,CumulativePixels,Percent,CumulativePercent

Is this a bug in using output attributes with PDS ingestion apps?

The crux of the problem is how the offsets and base are being initiated

    Group = Pixels
      Type       = SignedWord
      ByteOrder  = Lsb
      Base       = -1.79769313486231e+308
      Multiplier = 0.0
    End_Group

I'll keep looking into it but throwing it out in case anyone has an idea of what might be happening

acpaquette · 2021-06-02T20:10:09Z

@Kelvinrr Have you tried with a +8 or +32 and seeing what the output looks like? I agree that this may be an ingestion app problem but I figure the cube attributes "should just propogate" but I don't know

Kelvinrr · 2021-06-02T20:12:55Z

+8 is also garbage, +32 is fine as +32 = Real. +8 or +16 without setting min and max as well through the output attributes basically gives 0 valid pixels.

acpaquette · 2021-06-02T20:15:59Z

Maybe this wasn't a bug and is actually how the images need to be ingested? The only other thing I can think to try is to ingest some other PDS image, setting the output bit type and seeing what happens

Kelvinrr · 2021-06-02T20:16:13Z

isis/tests/FunctionalTestsThm2isis.cpp

+  // Pixels Group
+  EXPECT_EQ(PixelTypeName(evenCube.pixelType()).toStdString(), "UnsignedByte");
+  EXPECT_EQ(ByteOrderName(evenCube.byteOrder()).toStdString(), "Msb");
+  EXPECT_DOUBLE_EQ(evenCube.base(), -0.0039525691699605001);
+  EXPECT_DOUBLE_EQ(evenCube.multiplier(), 0.0039525691699605001);
+
+  std::unique_ptr<Histogram> hist (evenCube.histogram());
+
+  EXPECT_NEAR(hist->Minimum(), 0.0, 0.0001);
+  EXPECT_NEAR(hist->Maximum(), 0.0, 0.0001); 
+  EXPECT_NEAR(hist->Average(), 0.0, 0.0001);
+  EXPECT_NEAR(hist->Sum(), 0.0, .00001);
+  EXPECT_EQ(hist->ValidPixels(), 188100);
+  EXPECT_NEAR(hist->StandardDeviation(), 0, .00001);


You can see it here, I set a min and max to avoid all invalid pixels and ended up with 0 pixels only.

isis/tests/FunctionalTestsThm2isis.cpp

acpaquette · 2021-06-02T20:42:23Z

Looks like the test image wasn't cropped potentially?

Kelvinrr · 2021-06-02T20:49:27Z

@acpaquette Sorry, missed that comment on different data sets. I tried 3 different images so far (mix of vis and infrared) and they all output the same.

I don't think there is a way to know min/max programmatically, I can look into the logic of how the output attributes are actually being used because I got a feeling it should be doing that and it's not.

I didn't crop the one in there as it's already fairly small at ~5mb, but if that seems to big I can go back to seeing how to make it smaller.

acpaquette

Overall looks good! A few things that could be addressed

acpaquette · 2021-06-04T18:36:42Z

isis/src/odyssey/apps/thm2isis/thm2isis.cpp

+#include "ProcessByBrick.h"
+#include "Brick.h"
+#include "OriginalLabel.h"
+


Doesn't this need an #include "thm2isis.h"?

acpaquette · 2021-06-04T18:40:36Z

isis/tests/FunctionalTestsThm2isis.cpp

+TEST_F(TempTestingFiles, FunctionalTestThm2isisOutAttributes) {
+  // tempDir exists if the fixture subclasses TempTestingFiles, which most do
+  QString outCubeFileName = tempDir.path() + "/test.cub+msb+8bit+0.0012:0.0013";
+  QVector<QString> args = {"from=data/thm2isis/V00821003RDR.QUB",  "to="+outCubeFileName};


Could we crop this cube for consistance?

acpaquette

Looks good!

* thm2isis now uses out attrs and converted to func * removed commented code that manually set pixel type * added gtests * better params * IR test added * reordered things * cropped images * cropped image for outattr tests too

* Change measure log name to be case-insensitive (#4441) * Lrowac2pds App and Test Conversion (#4429) * lrowac2pds app conversion * Lrowac2pds App and Test Conversion * Set global variables to static to prevent test failures * Gui Parameters now right aligned (#4507) * Findgaps gtests (#4422) * first few test cases, needs work * two working tests * add test for correlation tolerance * improve test data creation code * updated mapcam makefile (#4498) * merge conflict with 561e21e in changelog * improve grid extend test case (#4506) * fix sample_bits pvl output (#4500) * map2map conversion (#4435) * added code for two tests * test modifications * changed crop input and ouput function * fixed last two tests * added more tests * converted map2map * added conversion code for map2map * bring crop tests up to date with dev * added modifications to map2map and tests * final changes for map2map tests * Converted app and default test * Converted tests * Added histograms to tests. * Rename FunctionalTestsMap2Map.cpp to FunctionalTestsMap2map.cpp Co-authored-by: Amy Stamile <[email protected]> Co-authored-by: Amy Stamile <[email protected]> * thm2isis now uses out attrs and converted to func (#4504) * thm2isis now uses out attrs and converted to func * removed commented code that manually set pixel type * added gtests * better params * IR test added * reordered things * cropped images * cropped image for outattr tests too * added missing changelog lines * Fix caminfo uselabel SegFault (#4402) * conditionally reopen cube to prevent segfault * relocate cube close * version ticks * change log merge conflict * added 5.0.1 bugfixes under a 5.0.1 header in CHANGELOG * removed unreleased portion of the changelog Co-authored-by: Jesse Mapel <[email protected]> Co-authored-by: Amy Stamile <[email protected]> Co-authored-by: Tim Giroux <[email protected]> Co-authored-by: robotprogrammer22 <[email protected]> Co-authored-by: Amy Stamile <[email protected]>

thm2isis now uses out attrs and converted to func

dad2f9b

Kelvinrr commented Jun 2, 2021

View reviewed changes

Kelvinrr requested a review from acpaquette June 2, 2021 17:15

Kelvinrr marked this pull request as draft June 2, 2021 17:16

removed commented code that manually set pixel type

4df57b6

acpaquette reviewed Jun 2, 2021

View reviewed changes

added gtests

8775c57

Kelvinrr commented Jun 2, 2021

View reviewed changes

better params

a1d38d5

Kelvinrr commented Jun 2, 2021

View reviewed changes

isis/tests/FunctionalTestsThm2isis.cpp Show resolved Hide resolved

IR test added

fa25400

Kelvinrr marked this pull request as ready for review June 3, 2021 04:32

reordered things

a7ac136

acpaquette reviewed Jun 4, 2021

View reviewed changes

Kelvin added 2 commits June 4, 2021 14:45

cropped images

bbfb2f8

cropped image for outattr tests too

151612b

acpaquette approved these changes Jun 4, 2021

View reviewed changes

Kelvinrr merged commit 4459b14 into DOI-USGS:dev Jun 5, 2021

jessemapel mentioned this pull request Jun 9, 2021

Thm2isis not properly propagating cube attributes #4213

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

thm2isis now uses out attrs and converted to func #4504

thm2isis now uses out attrs and converted to func #4504

Kelvinrr commented Jun 2, 2021

Kelvinrr Jun 2, 2021 •

edited

Loading

acpaquette left a comment

Kelvinrr commented Jun 2, 2021

Kelvinrr commented Jun 2, 2021

acpaquette commented Jun 2, 2021

Kelvinrr commented Jun 2, 2021 •

edited

Loading

acpaquette commented Jun 2, 2021

Kelvinrr Jun 2, 2021

acpaquette commented Jun 2, 2021

Kelvinrr commented Jun 2, 2021 •

edited

Loading

acpaquette left a comment

acpaquette Jun 4, 2021

acpaquette Jun 4, 2021

acpaquette left a comment

thm2isis now uses out attrs and converted to func #4504

thm2isis now uses out attrs and converted to func #4504

Conversation

Kelvinrr commented Jun 2, 2021

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Licensing

Kelvinrr Jun 2, 2021 • edited Loading

Choose a reason for hiding this comment

acpaquette left a comment

Choose a reason for hiding this comment

Kelvinrr commented Jun 2, 2021

Kelvinrr commented Jun 2, 2021

acpaquette commented Jun 2, 2021

Kelvinrr commented Jun 2, 2021 • edited Loading

acpaquette commented Jun 2, 2021

Kelvinrr Jun 2, 2021

Choose a reason for hiding this comment

acpaquette commented Jun 2, 2021

Kelvinrr commented Jun 2, 2021 • edited Loading

acpaquette left a comment

Choose a reason for hiding this comment

acpaquette Jun 4, 2021

Choose a reason for hiding this comment

acpaquette Jun 4, 2021

Choose a reason for hiding this comment

acpaquette left a comment

Choose a reason for hiding this comment

Kelvinrr Jun 2, 2021 •

edited

Loading

Kelvinrr commented Jun 2, 2021 •

edited

Loading

Kelvinrr commented Jun 2, 2021 •

edited

Loading