ortk95 / planetmapper

PlanetMapper: An open source Python package for visualising, navigating and mapping Solar System observations
https://planetmapper.readthedocs.io
MIT License
10 stars 1 forks source link

Allow 'surface' altitude to be customised #350

Closed ortk95 closed 2 months ago

ortk95 commented 4 months ago

We should add an optional offset to the altitude of the target body's 'surface' level, e.g. to allow mapping of atmospheric structure above/below the nominal 1-bar 'surface'.

~Should be able to implement this with a new kwarg in Body, which then adjusts the r_eq and r_polar values accordingly in __init__. I think everything should then flow automatically from this (mapping, wireframes, coordinate conversions etc.), though obviously need to check this.~

ortk95 commented 3 months ago

Doing this per-object looks like it'll end up quite complicated (and potentially fragile). I think it will need a whole new target creating dynamically in __init__, with different radii, and all associated kernels, IDs, variables etc. created for each new radius.

It will probably therefore be easier to implement this within each method by adding an alt parameter to any functions which convert to lonlats - i.e. this basically completes the work of #359 (which added an alt parameter for conversions from lonlats). I think the best way to do this will be to define a context manager that temporarily adjusts the kernel variables containing the radii (e.g. BODY599_RADII) when needed. For example, something like:

class AdjustAltitude:
    def __init__(self, target_id: int, alt: float):
        self.target_id = target_id
        self.alt = alt

    def __enter__(self):
        self._original_radii = spice.bodvar(self.target_id, 'RADII', 3)
        spice.pdpool(
            f'BODY{self.target_id}_RADII', [r + self.alt for r in self._original_radii]
        )

    def __exit__(self, exc_type, exc_val, exc_tb):
        spice.pdpool(f'BODY{self.target_id}_RADII', self._original_radii)

def radec2lonlat(ra: float, dec: float, *, alt: float = 0.0) -> tuple[float, float]:
    with AdjustAltitude(code, alt):
        ...

Would obviously need to test this properly to ensure the variables are changed correctly, in particular ensuring that the radius value in the main kernel pool isn't ever left with an adjusted value. Would probably want to add some short-circuit logic for the alt=0 case.

ortk95 commented 2 months ago

Performance tests

Testing code timings with repeats=100 and number=100

  TIME (s)  |              COMMAND               
7.46801e-05 | body.lonlat2radec(lon, lat)
7.92263e-05 | body.lonlat2radec(lon, lat, alt=1)
1.74475e-04 | body.radec2lonlat(ra, dec)
2.69785e-04 | body.radec2lonlat(ra, dec, alt=1)

body.lonlat2radec(lon, lat)       |7.47e-05|████████▎
body.lonlat2radec(lon, lat, alt=1)|7.92e-05|████████▊
body.radec2lonlat(ra, dec)        |1.74e-04|███████████████████▍
body.radec2lonlat(ra, dec, alt=1) |2.70e-04|██████████████████████████████

As expected, lonlat2radec has no difference (as it is just a change in the the length of a targvec), while radec2lonlat has some performance penalty from using the context manager. The 50% difference is fine though, as this is a relatively fast function.

Testing code timings with repeats=100 and number=10

  TIME (s)  |                COMMAND                 
4.90622e-02 | body.limb_radec_by_illumination()
5.04772e-02 | body.limb_radec_by_illumination(alt=1)

body.limb_radec_by_illumination()     |4.91e-02|█████████████████████████▎
body.limb_radec_by_illumination(alt=1)|5.05e-02|██████████████████████████
Testing code timings with repeats=10 and number=10

  TIME (s)  |                     COMMAND                     
9.30006e-02 | cc(); body.get_backplane_img("emission")
8.53116e-02 | cc(); body.get_backplane_img("emission", alt=1)

cc(); body.get_backplane_img("emission", alt=1)|8.53e-02|███████████████▋
cc(); body.get_backplane_img("emission")       |9.30e-02|█████████████████

# cc = lambda: body._clear_cache()
Testing code timings with repeats=10 and number=10

  TIME (s)  |                     COMMAND                     
8.63041e-05 | cc(); body.get_backplane_map("emission")
7.09200e-05 | cc(); body.get_backplane_map("emission", alt=1)

cc(); body.get_backplane_map("emission", alt=1)|7.09e-05|██████████████
cc(); body.get_backplane_map("emission")       |8.63e-05|█████████████████

Once you apply an alt adjustment to a bigger function, the O(1) altitude adjustment overhead becomes negligible compared to random timing noise in the O(n) transformations :)