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

Input issues with sunpy.physics.differential_rotation.solar_rotate_coordinate #7185

Open
hayesla opened this issue Sep 8, 2023 · 4 comments
Labels
Bug Probably a bug Effort Low Requires a small time investment Good First Issue The best issues for new people to tackle! Package Novice Requires little knowledge of the internal structure of SunPy Priority Medium Non-urgent action required

Comments

@hayesla
Copy link
Member

hayesla commented Sep 8, 2023

Describe the bug

I've been playing around with solar_rotate_coordinate and came across some issue s with inputs passed.

1. Issues with obstime of coordinate passed:

If you create a coordinate and dont specifically pass an obstime (as its in the observer coordinate in the frame) you get an error such as:

>>> earth_coord =get_earth(Time("2022-03-30"))
>>> coord_hpc = SkyCoord(100*u.arcsec, 100*u.arcsec, frame=frames.Helioprojective(observer=earth_coord))
>>> solar_rotate_coordinate(coord_hpc, time=Time("2022-03-31"))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[40], line 1
----> 1 solar_rotate_coordinate(coord_hpc, time=Time("2022-03-31"))

File ~/opt/miniconda3/envs/stixflarelist/lib/python3.11/site-packages/sunpy/physics/differential_rotation.py:288, in solar_rotate_coordinate(coordinate, observer, time, **diff_rot_kwargs)
    285 diff_rot_kwargs.update({"frame_time": "sidereal"})
    287 # Calculate the interval between the start and end time
--> 288 interval = (new_observer.obstime - coordinate.obstime).to(u.s)
    290 # Compute Stonyhurst Heliographic coordinates - returns (longitude,
    291 # latitude). Points off the limb are returned as nan.
    292 heliographic_coordinate = coordinate.transform_to(HeliographicStonyhurst)

TypeError: unsupported operand type(s) for -: 'Time' and 'NoneType'

however, of you pass obstime in the coordinate frame it doesnt have this issue:

>>> earth_coord =get_earth(Time("2022-03-30"))
>>> coord_hpc = SkyCoord(100*u.arcsec, 100*u.arcsec, frame=frames.Helioprojective(observer=earth_coord, obstime=earth_coord.obstime))
>>> solar_rotate_coordinate(coord_hpc, time=Time("2022-03-31"))
<SkyCoord (Helioprojective: obstime=2022-03-31 00:00:00.000, rsun=695700.0 km, observer=<HeliographicStonyhurst Coordinate (obstime=2022-03-31 00:00:00.000, rsun=695700.0 km): (lon, lat, radius) in (deg, deg, AU)
    (0., -6.61102453, 0.99881835)>): (Tx, Ty, distance) in (arcsec, arcsec, AU)
    (318.46860338, 93.47601016, 0.99445314)>

so shouldn't solar_rotate_coordinate just take the observer coordinate obstime if the attribute isnt on the coordinate?

2. Issue when passing coordinate as HGS

Also there seems to be an issue if you pass a HGS coordinate instead of a HPC coord:

coord_hgs = coord_hpc.transform_to(frames.HeliographicStonyhurst)
solar_rotate_coordinate(coord_hgs, time=Time("2022-03-31"))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[49], line 2
      1 coord_hgs = coord_hpc.transform_to(frames.HeliographicStonyhurst)
----> 2 solar_rotate_coordinate(coord_hgs, time=Time("2022-03-31"))

File ~/opt/miniconda3/envs/stixflarelist/lib/python3.11/site-packages/sunpy/physics/differential_rotation.py:307, in solar_rotate_coordinate(coordinate, observer, time, **diff_rot_kwargs)
    298 heliographic_rotated = SkyCoord(heliographic_coordinate.lon + drot,
    299                                 heliographic_coordinate.lat,
    300                                 heliographic_coordinate.radius,
    301                                 obstime=coordinate.obstime,
    302                                 frame=HeliographicStonyhurst)
    304 # Calculate where the rotated coordinate appears as seen by new observer
    305 # for the coordinate system of the input coordinate.  The translational
    306 # motion of the Sun will be ignored for the transformation.
--> 307 frame_newobs = coordinate.frame.replicate_without_data(observer=new_observer,
    308                                                        obstime=new_observer.obstime)
    309 with transform_with_sun_center():
    310     return heliographic_rotated.transform_to(frame_newobs)

File ~/opt/miniconda3/envs/stixflarelist/lib/python3.11/site-packages/astropy/coordinates/baseframe.py:1017, in BaseCoordinateFrame.replicate_without_data(self, copy, **kwargs)
    990 def replicate_without_data(self, copy=False, **kwargs):
    991     """
    992     Return a replica without data, optionally with new frame attributes.
    993 
   (...)
   1015         attributes.
   1016     """
-> 1017     return self._replicate(None, copy=copy, **kwargs)

File ~/opt/miniconda3/envs/stixflarelist/lib/python3.11/site-packages/astropy/coordinates/baseframe.py:959, in BaseCoordinateFrame._replicate(self, data, copy, **kwargs)
    955             value = value.copy()
    957         kwargs[attr] = value
--> 959 return self.__class__(data, copy=False, **kwargs)

File ~/opt/miniconda3/envs/stixflarelist/lib/python3.11/site-packages/sunpy/coordinates/frames.py:136, in SunPyBaseCoordinateFrame.__init__(self, *args, **kwargs)
    133 if not kwargs.pop('wrap_longitude', True):
    134     self._wrap_angle = None
--> 136 super().__init__(*args, **kwargs)
    138 # If obstime is specified, treat the default observer (None) as explicitly set
    139 if self.obstime is not None and self.is_frame_attr_default('observer'):

File ~/opt/miniconda3/envs/stixflarelist/lib/python3.11/site-packages/astropy/coordinates/baseframe.py:336, in BaseCoordinateFrame.__init__(self, copy, representation_type, differential_type, *args, **kwargs)
    333         self._attr_names_with_defaults.append(fnm)
    335 if kwargs:
--> 336     raise TypeError(
    337         f"Coordinate frame {self.__class__.__name__} got unexpected "
    338         f"keywords: {list(kwargs)}"
    339     )
    341 # We do ``is None`` because self._data might evaluate to false for
    342 # empty arrays or data == 0
    343 if self._data is None:
    344     # No data: we still need to check that any non-scalar attributes
    345     # have consistent shapes. Collect them for all attributes with
    346     # size > 1 (which should be array-like and thus have a shape).

TypeError: Coordinate frame HeliographicStonyhurst got unexpected keywords: ['observer']
@ayshih
Copy link
Member

ayshih commented Sep 8, 2023

  1. That's really a feature request rather than a bug. See When instantiating a frame, use obstime from observer if obstime not otherwise provided #7186.
  2. We aren't particularly interested in fixing issues with solar_rotate_coordinate(). Can you use propagate_with_solar_surface() instead?
>>> with propagate_with_solar_surface():
...     coord_hgs.transform_to(frames.HeliographicStonyhurst(obstime='2022-03-31'))

@hayesla
Copy link
Member Author

hayesla commented Sep 8, 2023

thanks @ayshih

for 1. great - missed that PR.

for 2. absolutely, so is the issue that solar_rotate_coordinate() is going to be deprecated? at the moment, its broken right if you try to do this?

@ayshih
Copy link
Member

ayshih commented Sep 8, 2023

for 1. great - missed that PR.

Heh, you didn't miss anything. I created that PR in response to this issue.

for 2. absolutely, so is the issue that solar_rotate_coordinate() is going to be deprecated? at the moment, its broken right if you try to do this?

The bug is straightforward to fix: it shouldn't specify observer for the final frame if the final frame doesn't use that attribute. solar_rotate_coordinate() isn't necessarily going to be deprecated, but the intent is to rewrite its internals, which is why I'm not enthusiastic about fixing such bugs in the current implementation.

@nabobalis nabobalis added Package Novice Requires little knowledge of the internal structure of SunPy Priority Medium Non-urgent action required Effort Low Requires a small time investment Good First Issue The best issues for new people to tackle! labels Mar 20, 2024
@Deus1704
Copy link
Contributor

Deus1704 commented Mar 24, 2024

The bug is straightforward to fix: it shouldn't specify observer for the final frame if the final frame doesn't use that attribute.

Correct me if I'm wrong, but this is what I understood from the discussion above;
So the issue is that the solar_rotate_coordinate function by default replicates the attributes of the input frame for the coordinates in final output frame. Since in this case our input frame was HGS, it didn't have any observer attribute, which is causing the type error. This could be fixed by just having a check before hand if the input frame has an observer attribute or not?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Probably a bug Effort Low Requires a small time investment Good First Issue The best issues for new people to tackle! Package Novice Requires little knowledge of the internal structure of SunPy Priority Medium Non-urgent action required
Projects
None yet
Development

No branches or pull requests

4 participants