pingswept / pysolar

Pysolar is a collection of Python libraries for simulating the irradiation of any point on earth by the sun. It includes code for extremely precise ephemeris calculations.
http://pysolar.org
GNU General Public License v3.0
372 stars 125 forks source link

BUG: get_altitude() doesn't *actually* take into account elevation #154

Closed jhaskinsPhD closed 1 year ago

jhaskinsPhD commented 1 year ago

The function doesn't pass the input argument for elevation to the function, get_topocentric_position(). So the capability is here, just need to add elevation=elevation to line 125.

Currently:

def get_altitude(latitude_deg, longitude_deg, when, elevation = 0,
                 temperature = constants.standard_temperature, pressure = constants.standard_pressure):
    '''See also the faster, but less accurate, get_altitude_fast()
    temperature in Kelvin and pressure in Pascal
    '''
    topocentric_sun_declination, topocentric_local_hour_angle = \
        get_topocentric_position(latitude_deg, longitude_deg, when)

    topocentric_elevation_angle = get_topocentric_elevation_angle(latitude_deg, topocentric_sun_declination, topocentric_local_hour_angle)
    refraction_correction = get_refraction_correction(pressure, temperature, topocentric_elevation_angle)
    return topocentric_elevation_angle + refraction_correction

Should be:

def get_altitude(latitude_deg, longitude_deg, when, elevation = 0,
                 temperature = constants.standard_temperature, pressure = constants.standard_pressure):
    '''See also the faster, but less accurate, get_altitude_fast()
    temperature in Kelvin and pressure in Pascal
    '''
    topocentric_sun_declination, topocentric_local_hour_angle = \
        get_topocentric_position(latitude_deg, longitude_deg, when, elevation=elevation)

    topocentric_elevation_angle = get_topocentric_elevation_angle(latitude_deg, topocentric_sun_declination, topocentric_local_hour_angle)
    refraction_correction = get_refraction_correction(pressure, temperature, topocentric_elevation_angle)
    return topocentric_elevation_angle + refraction_correction
jhaskinsPhD commented 1 year ago

Also worth adding to all the documentation that elevation is assumed to be in meters (from what I can tell)?

pingswept commented 1 year ago

That seems like a reasonable fix. If you (or anyone else) can submit a pull request that fixes L125 in solar.py as you’ve shown, I would happily accept it. Otherwise, I’ll make one when I get a chance.

pingswept commented 1 year ago

This is now patched. Thanks for the help, @jhaskinsPhD and @obsoletesystm.

darksidelemm commented 1 year ago

When will the next release be? It would be nice to have this fix available in pip, since I just hit this bug myself.

pingswept commented 1 year ago

New release, 0.11, just uploaded to PyPI. Let me know if anything seems broken.