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

Refactor lrowaccal for testability and add a radiance units label to the output cube #4940

Merged
merged 27 commits into from
May 11, 2022

Conversation

cordellmichaud
Copy link
Contributor

@cordellmichaud cordellmichaud commented May 4, 2022

Description

  • Removed old lrowaccal Makefile tests
  • Refactored lrowaccal to be callable for testing, following the procedure in the Writing ISIS Tests Using Gtest and Ctest section of the wiki
  • Moved previously global variables into new callable lrowaccal function
  • Converted lrowaccal helper functions into lambdas with captures
  • Added units label "W/m2/sr/um" to the RadiometricType keyword of the lrowaccal output cube label when the RadiometricType parameter is Radiance
  • Added functional tests for lrowaccal to test units label change
  • Updated lrowaccal xml file history section with changes

Related Issue

#4939

Motivation and Context

This change adds a radiance units label to the output cube of lrowaccal as suggested by the LROC science team to more clearly indicate the radiance units and accompany a change to the radiance calibration coefficients to make the radiance units match those of the NAC.

This change also brings lrowaccal in line with the newer testing framework for ISIS and covers the units label change with functional tests.

How Has This Been Tested?

Functional tests were added to isis/tests/FunctionalTestsLrowaccal.cpp to cover the units label change and ensure the label is only added when the RadiometricType parameter is Radiance.

The change was also tested and validated by the science team at LROC alongside the change to the radiance calibration coefficients.

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.

cordellmichaud and others added 20 commits March 23, 2022 13:24
Add radiance units label in lrowaccal output cube
- Merge branch 'dev-lrowaccal-unitsfix-tests' of https://github.com/lrocdev/ISIS3 into dev-lrowaccal-unitsfix-tests
isis/src/lro/apps/lrowaccal/lrowaccal.h Show resolved Hide resolved
isis/tests/FunctionalTestsLrowaccal.cpp Outdated Show resolved Hide resolved
isis/tests/FunctionalTestsLrowaccal.cpp Outdated Show resolved Hide resolved
isis/tests/FunctionalTestsLrowaccal.cpp Show resolved Hide resolved
isis/src/lro/apps/lrowaccal/main.cpp Outdated Show resolved Hide resolved
isis/src/lro/apps/lrowaccal/main.cpp Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
isis/src/lro/apps/lrowaccal/lrowaccal.cpp Outdated Show resolved Hide resolved
isis/src/lro/apps/lrowaccal/lrowaccal.cpp Show resolved Hide resolved
Copy link
Contributor

@jessemapel jessemapel left a comment

Choose a reason for hiding this comment

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

Migration and new test look good.

@AustinSanders AustinSanders merged commit 1facdb7 into DOI-USGS:dev May 11, 2022
Kelvinrr pushed a commit to gsn9/ISIS3 that referenced this pull request Jul 18, 2022
…the output cube (DOI-USGS#4940)

* Add radiance units label in lrowaccal output cube

* Refactor lrowaccal app to be callable for testing and add radiance units to output PVL

* Update formatting and NULL initialization

* Add NULL assignments after deletions

* Add CubeAttributeOutput to output cube initialization

* Add functional test for lrowaccal units label addition and accompanying test cube

* Refactor lrowaccal to remove global variables and convert helper functions to lambdas with captures

* Update history section of lrowaccal XML file with refactoring and output cube units label changes

* Remove old lrowaccal Makefile tests

* Add lrowaccal functional test to ensure that the radiance units label is not inserted for RadiometricType IOF

* Add Cordell Michaud to the .zenodo.json document

* Add lrowaccal changes to CHANGELOG.md

* Clean up lrowaccal includes and leftover comment

* Re-add lrowaccal Makefile tests

* Re-add disclaimer to top of lrowaccal's main.cpp file

* Add input attributes retrieval to lrowaccal.cpp

* Fix lrowaccal functional test error messages and add PVl group and existence checks

* Move lrowaccal units change message to Changed sub-header in CHANGELOG.md

* Simplify lrowaccal functional test assertions

Co-authored-by: Victor Silva <victor.silva@asu.edu>
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.

4 participants