From 163d87d1e3cd285aa54fb59ffa04e88ce2eeb975 Mon Sep 17 00:00:00 2001 From: Trent Hare Date: Thu, 18 Aug 2022 13:39:07 -0700 Subject: [PATCH 1/5] Update to isd_generate.py to allow for a NAIF radius override Based on this RFC, we want to change our Mars pipeline to use a sphere. https://astrodiscuss.usgs.gov/t/appl-rfc-use-iau-sphere-for-hirise-dtm-projects-in-socet-set/428 Currently ALE uses the Mars elliptical defaults from the NAIF kernels. This update allows the user to override the kernels and set a spherical radius for both semimajor and semiminor in the ISD. This is not only needed for Mars. For triaxial bodies, the IAU recommends a best-fit sphere should also be set for map products. References (DOI) are listed in the updated help for the current IAU values. --- ale/isd_generate.py | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/ale/isd_generate.py b/ale/isd_generate.py index f991e2fb7..e1f67897c 100755 --- a/ale/isd_generate.py +++ b/ale/isd_generate.py @@ -16,7 +16,7 @@ import os from pathlib import Path import sys - +import json import ale logger = logging.getLogger(__name__) @@ -51,6 +51,19 @@ def main(): "and the default strategy of replacing their final suffix with " ".json will be used to generate the output file paths." ) + parser.add_argument( + "-r", "--radius", + type=float, + default=None, + help="Optional spherical radius (km) override. If not specified, the default " + "radius values, as defined in the NAIF kernels, will be used. " + "There often cases were the defined NAIF radii are set as a triaxial " + "when the IAU WGCCRE report recommends a best-fit sphere for a " + "derived map product. For current IAU spherical recommendations see: " + "https://doi.org/10.1007/s10569-017-9805-5 or " + "http://voparis-vespa-crs.obspm.fr:8080/web/. " + "Make sure radius value is in kilometers." + ) parser.add_argument( "-v", "--verbose", action="store_true", @@ -84,9 +97,12 @@ def main(): except KeyError: k = [args.kernel, ] + if args.radius is not None: + rad = args.radius + if len(args.input) == 1: try: - file_to_isd(args.input[0], args.out, kernels=k, log_level=log_level) + file_to_isd(args.input[0], args.out, rad, kernels=k, log_level=log_level) except Exception as err: # Seriously, this just throws a generic Exception? sys.exit(f"File {args.input[0]}: {err}") @@ -96,7 +112,7 @@ def main(): ) as executor: futures = { executor.submit( - file_to_isd, f, **{"kernels": k, "log_level": log_level} + file_to_isd, f, **{"radius": rad, "kernels": k, "log_level": log_level} ): f for f in args.input } for f in concurrent.futures.as_completed(futures): @@ -113,6 +129,7 @@ def main(): def file_to_isd( file: os.PathLike, out: os.PathLike = None, + radius: float = None, kernels: list = None, log_level=logging.WARNING ): @@ -143,6 +160,15 @@ def file_to_isd( else: usgscsm_str = ale.loads(file) + if radius is not None: + usgscsm_json = json.loads(usgscsm_str) + usgscsm_json["radii"]["semimajor"] = radius + usgscsm_json["radii"]["semiminor"] = radius + logger.info(f"Overriding radius to:") + logger.info(usgscsm_json["radii"]) + usgscsm_str = json.dumps(usgscsm_json, indent=2) + isd_file.write_text(usgscsm_str) + logger.info(f"Writing: {isd_file}") isd_file.write_text(usgscsm_str) From 331778fcc31974fca963bb69d3fc5b327e88a337 Mon Sep 17 00:00:00 2001 From: Trent Hare Date: Thu, 18 Aug 2022 14:01:52 -0700 Subject: [PATCH 2/5] remove extra file write if new radius is sent --- ale/isd_generate.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ale/isd_generate.py b/ale/isd_generate.py index e1f67897c..2523aaaf3 100755 --- a/ale/isd_generate.py +++ b/ale/isd_generate.py @@ -167,7 +167,6 @@ def file_to_isd( logger.info(f"Overriding radius to:") logger.info(usgscsm_json["radii"]) usgscsm_str = json.dumps(usgscsm_json, indent=2) - isd_file.write_text(usgscsm_str) logger.info(f"Writing: {isd_file}") isd_file.write_text(usgscsm_str) From f29fe975979d4c5d5e67c33aee1568065d2ed645 Mon Sep 17 00:00:00 2001 From: Trent Hare Date: Fri, 19 Aug 2022 18:22:51 -0700 Subject: [PATCH 3/5] update to allow ellipse, minor help update Add --semimajor and --semiminor parameters. I tried nargs="+" for --radius but fought for too long with argparse to do all the checking (like checking it is float). I am fine with these options but happy to hear better ways to use argparse. --- ale/isd_generate.py | 52 +++++++++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/ale/isd_generate.py b/ale/isd_generate.py index 2523aaaf3..974f2e4cb 100755 --- a/ale/isd_generate.py +++ b/ale/isd_generate.py @@ -52,17 +52,32 @@ def main(): ".json will be used to generate the output file paths." ) parser.add_argument( - "-r", "--radius", + "--semimajor", + required='--semiminor' in sys.argv, type=float, default=None, - help="Optional spherical radius (km) override. If not specified, the default " - "radius values, as defined in the NAIF kernels, will be used. " - "There often cases were the defined NAIF radii are set as a triaxial " - "when the IAU WGCCRE report recommends a best-fit sphere for a " - "derived map product. For current IAU spherical recommendations see: " + help="Optional spherical radius (km) override. Setting " + " '--semimajor 3396.19' " + "will override both semi-major and semi-minor radius values with the same value. " + "An ellipse can be defined if '--semiminor' is also sent. " + "If not specified, the default radius " + "values (e.g.; from NAIF kernels or the ISIS Cube) will be used. " + "When is needed? Beyond a specialized need, it is common " + "that planetary bodies are defined as a triaxial body. " + "In most of these cases, the IAU WGCCRE report recommends the use of a " + "best-fit sphere for a derived map product. " + "For current IAU spherical recommendations see: " "https://doi.org/10.1007/s10569-017-9805-5 or " - "http://voparis-vespa-crs.obspm.fr:8080/web/. " - "Make sure radius value is in kilometers." + "http://voparis-vespa-crs.obspm.fr:8080/web/ " + "Make sure radius values are in kilometers." + ) + parser.add_argument( + "--semiminor", + type=float, + default=None, + help="Optional semi-minor radius (km) override. When using this parameter, you must also define the semi-major radius. Setting " + " '--semimajor 3396.19 --semiminor 3376.2' " + "will override the semi-major and semi-minor radii to define an ellipse. " ) parser.add_argument( "-v", "--verbose", @@ -97,12 +112,17 @@ def main(): except KeyError: k = [args.kernel, ] - if args.radius is not None: - rad = args.radius + if args.semimajor is None: + radii = None + else: + if args.semiminor is None: # set a sphere + radii = [args.semimajor, args.semimajor] + else: # set as ellipse + radii = [args.semimajor, args.semiminor] if len(args.input) == 1: try: - file_to_isd(args.input[0], args.out, rad, kernels=k, log_level=log_level) + file_to_isd(args.input[0], args.out, radii, kernels=k, log_level=log_level) except Exception as err: # Seriously, this just throws a generic Exception? sys.exit(f"File {args.input[0]}: {err}") @@ -112,7 +132,7 @@ def main(): ) as executor: futures = { executor.submit( - file_to_isd, f, **{"radius": rad, "kernels": k, "log_level": log_level} + file_to_isd, f, **{"radii": radii, "kernels": k, "log_level": log_level} ): f for f in args.input } for f in concurrent.futures.as_completed(futures): @@ -129,7 +149,7 @@ def main(): def file_to_isd( file: os.PathLike, out: os.PathLike = None, - radius: float = None, + radii: list = None, kernels: list = None, log_level=logging.WARNING ): @@ -160,10 +180,10 @@ def file_to_isd( else: usgscsm_str = ale.loads(file) - if radius is not None: + if radii is not None: usgscsm_json = json.loads(usgscsm_str) - usgscsm_json["radii"]["semimajor"] = radius - usgscsm_json["radii"]["semiminor"] = radius + usgscsm_json["radii"]["semimajor"] = radii[0] + usgscsm_json["radii"]["semiminor"] = radii[1] logger.info(f"Overriding radius to:") logger.info(usgscsm_json["radii"]) usgscsm_str = json.dumps(usgscsm_json, indent=2) From 74bde41f20f9fbaa5b4f663be56e18bf64e6f220 Mon Sep 17 00:00:00 2001 From: Trent Hare Date: Fri, 25 Oct 2024 10:13:31 -0700 Subject: [PATCH 4/5] Update isd_generate.py (#619) add in PROJ like options for radius names ("-a", "-r", "--radius", "-b") --- ale/isd_generate.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ale/isd_generate.py b/ale/isd_generate.py index 974f2e4cb..8692db28a 100755 --- a/ale/isd_generate.py +++ b/ale/isd_generate.py @@ -52,8 +52,8 @@ def main(): ".json will be used to generate the output file paths." ) parser.add_argument( - "--semimajor", - required='--semiminor' in sys.argv, + "--semimajor", "-a", "-r", "--radius", + required="--semiminor" in sys.argv, type=float, default=None, help="Optional spherical radius (km) override. Setting " @@ -68,11 +68,11 @@ def main(): "best-fit sphere for a derived map product. " "For current IAU spherical recommendations see: " "https://doi.org/10.1007/s10569-017-9805-5 or " - "http://voparis-vespa-crs.obspm.fr:8080/web/ " + "http://voparis-vespa-crs.obspm.fr:8080/web/ ." "Make sure radius values are in kilometers." ) parser.add_argument( - "--semiminor", + "--semiminor", "-b", type=float, default=None, help="Optional semi-minor radius (km) override. When using this parameter, you must also define the semi-major radius. Setting " @@ -199,3 +199,4 @@ def file_to_isd( sys.exit(main()) except ValueError as err: sys.exit(err) + From 25e44f980edb956162f7b03b034784ea55df443f Mon Sep 17 00:00:00 2001 From: Trent Hare Date: Fri, 25 Oct 2024 14:26:18 -0700 Subject: [PATCH 5/5] Radius update with changes to address original PR and review comments (#620) * Update isd_generate.py add in PROJ like options for radius names ("-a", "-r", "--radius", "-b") * Update isd_generate.py updated required statement for --semimajor to catch if only --semiminor or -b is sent. Request that radius is sent in meters and updated some typos. --------- Co-authored-by: Austin Sanders --- ale/isd_generate.py | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/ale/isd_generate.py b/ale/isd_generate.py index 8692db28a..b343bde11 100755 --- a/ale/isd_generate.py +++ b/ale/isd_generate.py @@ -53,31 +53,31 @@ def main(): ) parser.add_argument( "--semimajor", "-a", "-r", "--radius", - required="--semiminor" in sys.argv, + required="--semiminor" in sys.argv or "-b" in sys.argv, type=float, default=None, - help="Optional spherical radius (km) override. Setting " - " '--semimajor 3396.19' " + help="Optional spherical radius (m) override. Setting " + " '--semimajor 3396190.0' " "will override both semi-major and semi-minor radius values with the same value. " - "An ellipse can be defined if '--semiminor' is also sent. " - "If not specified, the default radius " - "values (e.g.; from NAIF kernels or the ISIS Cube) will be used. " - "When is needed? Beyond a specialized need, it is common " + "An ellipsoid can be defined if '--semiminor' is also sent. " + "If not specified, the default radius values " + "(e.g.; from NAIF kernels or the ISIS Cube) will be used. " + "When is a semimajor specification needed? Beyond a specialized need, it is common " "that planetary bodies are defined as a triaxial body. " "In most of these cases, the IAU WGCCRE report recommends the use of a " "best-fit sphere for a derived map product. " "For current IAU spherical recommendations see: " "https://doi.org/10.1007/s10569-017-9805-5 or " "http://voparis-vespa-crs.obspm.fr:8080/web/ ." - "Make sure radius values are in kilometers." + "Make sure radius values are in meters (not kilometers)." ) parser.add_argument( "--semiminor", "-b", type=float, default=None, - help="Optional semi-minor radius (km) override. When using this parameter, you must also define the semi-major radius. Setting " - " '--semimajor 3396.19 --semiminor 3376.2' " - "will override the semi-major and semi-minor radii to define an ellipse. " + help="Optional semi-minor radius (m) override. When using this parameter, you must also define the semi-major radius. For example: " + " '--semimajor 3396190.0 --semiminor 3376200.0' " + "will override the semi-major and semi-minor radii to define an ellipsoid. " ) parser.add_argument( "-v", "--verbose", @@ -117,7 +117,7 @@ def main(): else: if args.semiminor is None: # set a sphere radii = [args.semimajor, args.semimajor] - else: # set as ellipse + else: # set as ellipsoid radii = [args.semimajor, args.semiminor] if len(args.input) == 1: @@ -181,10 +181,13 @@ def file_to_isd( usgscsm_str = ale.loads(file) if radii is not None: + # first convert to kilometers for ISD + radii = [x / 1000.0 for x in radii] + usgscsm_json = json.loads(usgscsm_str) usgscsm_json["radii"]["semimajor"] = radii[0] usgscsm_json["radii"]["semiminor"] = radii[1] - logger.info(f"Overriding radius to:") + logger.info(f"Overriding radius to (km):") logger.info(usgscsm_json["radii"]) usgscsm_str = json.dumps(usgscsm_json, indent=2)