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

Update the VARIABLE_MAP in the srml.py module #2356

Open
chzapo opened this issue Jan 14, 2025 · 8 comments
Open

Update the VARIABLE_MAP in the srml.py module #2356

chzapo opened this issue Jan 14, 2025 · 8 comments

Comments

@chzapo
Copy link

chzapo commented Jan 14, 2025

Via our discussion and recommendations on the discussion thread found here #2351 this issue is being submitted (first step in the pull request process).

The goal of this issue is to expand the VARIABLE_MAP to include other variables not included in pvlib-python as well as some of those that are included but never have been implemented.

Right now the srml.py module cannot be used as a general, stand alone tool for replacing the numbered column headers in an srml file with actual readable strings (only for a select 9 values that make up a small part of the total srml parsing set).

The full list can be found here and gives suggested naming conventions.
http://solardata.uoregon.edu/DataElementNumbers.html

A few iteration are planned on this to get the naming scheme correct.

Filling out the VARIABLE_MAP will allow the parser to act as a stand alone tool for porting out updated data files (which is our end use goal).

@chzapo
Copy link
Author

chzapo commented Jan 15, 2025

This is the structure I am thinking of (and comparison to the current structure)

VARIABLE_MAP = {
    '100': 'ghi',
    '201': 'dni',
    '300': 'dhi',
    '920': 'wind_direction',
    '921': 'wind_speed',
    '930': 'temp_air',
    '931': 'temp_dew',
    '933': 'relative_humidity',
    '937': 'temp_cell',
}
NEW_VARIABLE_MAP = {
'016': 'v_15_deg_south_tilt',
'024': 'v_25_deg_west_tilt',
'026': 'v_25_deg_west_tilt',
'028': 'v_25_deg_east_tilt',
'036': 'v_30_deg_south_tilt',
'056': 'v_45_deg_south_tilt',
'066': 'v_60_deg_south_tilt',
'100': 'ghi',
'109': 'gri',
'116': 'irr_15_deg_south_tilt',
'124': 'irr_25_deg_west_tilt',
'125': 'irr_25_deg_southwest_tilt',
'126': 'irr_25_deg_west_tilt',
'128': 'irr_25_deg_east_tilt',
'136': 'irr_30_deg_south_tilt',
'146': 'irr_35_deg_south_tilt',
'156': 'irr_45_deg_south_tilt',
'166': 'irr_60_deg_south_tilt',
'192': 'irr_90_deg_north_tilt',
'196': 'irr_90_deg_south_tilt',
'201': 'dni',
'300': 'dhi',
'416': 'i_15_deg_south_tilt',
'424': 'i_25_deg_west_tilt',
'426': 'i_25_deg_west_tilt',
'428': 'i_25_deg_east_tilt',
'436': 'i_30_deg_south_tilt',
'456': 'i_45_deg_south_tilt',
'466': 'i_60_deg_south_tilt',
'516': 'p_15_deg_south_tilt',
'524': 'p_25_deg_west_tilt',
'526': 'p_25_deg_west_tilt',
'528': 'p_25_deg_east_tilt',
'536': 'p_30_deg_south_tilt',
'556': 'p_45_deg_south_tilt',
'566': 'p_60_deg_south_tilt',
'616': 'v_15_deg_south_tilt',
'624': 'v_25_deg_west_tilt',
'626': 'v_25_deg_west_tilt',
'628': 'v_25_deg_east_tilt',
'636': 'v_30_deg_south_tilt',
'656': 'v_45_deg_south_tilt',
'666': 'v_60_deg_south_tilt',
'7000': 'hor_og_570',
'7001': 'hor_rg_630',
'7002': 'hor_rg_695',
'7003': 'hor_uva',
'7004': 'hor_ivb',
'7005': 'hor_zenith_illum',
'7006': 'hor_diffuse_illum',
'7007': 'hor_global_illum',
'7008': 'hor_max_illum',
'7009': 'hor_min_illum',
'7010': 'beam_og_570',
'7011': 'beam_rg_630',
'7012': 'beam_rg_695',
'7013': 'beam_uva',
'7014': 'beam_uvb',
'7017': 'beam_norm_illum',
'7018': 'beam_max_illum',
'7019': 'beam_min_illum',
'910': 'sky_condition',
'911': 'ceiling_height',
'912': 'visibility',
'913': 'weather',
'915': 'rainfall',
'917': 'bp',
'920': 'wind_direction',
'921': 'wind_speed',
'922': 'std_wind_direction',
'930': 'temp_air',
'931': 'temp_dew',
'933': 'relative_humidity',
'937': 'temp_cell',
'938': 'min_ambient_temp',
'939': 'max_ambient_temp',
'940': 'average_bp',
'951': 'total_sky_cover',
'952': 'opaque_sky_cover',
'953': 'precipitable_water',
'954': 'aerosol_optical_depth',
'955': 'snow_depth',
'956': 'albedo',
'965': 'days_last_snow',
}

It looks like I will need to update the test as well which I don't mind doing. I just need a yay or nay and some comments on the this naming scheme. I tried to follow the nomenclature where possible.
https://pvlib-python.readthedocs.io/en/stable/user_guide/nomenclature.html

We use bp internally at Sandia for barometric pressure. Hence the abbreviation there.

I think sometimes 'e' is used for irradiance, but wanted to confirm that before I used it on the tilt measurements.

I think 'gri' is the same as ' global ground facing irradiance', but correct me if that is wrong.

For the 7000s, I am not sure 'beam' is better than 'norm' in that prefix.

Let me know on anything else you see.

@chzapo
Copy link
Author

chzapo commented Jan 15, 2025

Already spotted an inconsistancy:
for 938 and 939 that probably should be
min_temp_air
max_temp_air

Is it better to put the min and max after the temp_air? Does pvlib tend to use
temp_air_max
temp_air_min
Instead?

acis.py also uses
snowdepth
instead of
snow_depth
will fix that in the next go around.

epw.py uses snow_depth so we probably need to fix that inconsistency.
That file also uses
days_since_last_snowfall

That files has a number of these column name replacements. Will update these.

@cwhanse
Copy link
Member

cwhanse commented Jan 16, 2025

For the tilted pyranometer measurements: I would use voltage_15_south or even voltage_15_180, with the pattern . I don't think 'deg' or 'tilt' are needed in the parameter name. Use 'voltage', 'current' and 'poa_irrad' although I'm not fond of 'poa_irrad' so open other other names.

I'm not sure what to recommend for the 7000s. What are these quantities?

If 'std_wind_direction' is the standard deviation of wind direction, I would put the modifier 'std' at the end.

@chzapo
Copy link
Author

chzapo commented Jan 16, 2025

For 7xxx, from the documentation:

"Spectral solar radiation data"
"7 — Spectral data (watt hours per square meter per hour, except for illuminance values which are kilolux hours per hour)"

I don't really have anymore insight than this.

I will implement your suggestions. Lets go with the voltage_15_180 format. This allows for the off chance that someone has a strange angle they want to use (127 degrees for example).

@echedey-ls
Copy link
Contributor

  1. snowfall as a parameter is already used two times in pvlib, so I midly prefer it over snow[_]depth.
  2. Instead of hor_, plain and full horizontal_ seems more readable to me. In general, in Python it's common to use full names instead of abbreviations for words.
  3. In addition to the voltage_ and current_ suggestion from @cwhanse , I'd use power_ instead of p_ for 5xxx — Power output of solar cell array (watts).
  4. What about poa_global_* instead of poa_irrad_*? It's in the Nomenclature page.
  5. Scipy and Pandas use std for standard deviation, statsmodels uses stdev. I prefer the latter, it's way easier to read, or just go with wind_direction_standard_deviation. I don't think many people will use it, so this makes it easier to discard it from a quick glance.

By the way, to format multiline code blocks, use triple back quotes (github docs)

@chzapo
Copy link
Author

chzapo commented Jan 16, 2025

I went ahead with implementing the changes on my own fork. I was not sure if it was ready to for a pull request.
main...chzapo:pvlib-python:main

I went with snow_depth because it appears that snowfall and snowdepth are actually two different measurements
https://github.com/pvlib/pvlib-python/blob/main/pvlib/iotools/acis.py
The acis.py iotool has both.

It could be that one is for real time snowfall conditions and the other is for accumulated snow fall conditions. I am not sure. I can look into it though.

I can modify it to horizontal_ if preferred I tend to be the type that writes very long variables in python. Usually people get annoyed by that. If that is the preference here all the better.

  1. Implemented.

  2. I already updated to poa_irrad_ but don't mind changing it. Just see what everyone wants.

  3. Okay I will look into this and see about a change.

@cwhanse
Copy link
Member

cwhanse commented Jan 16, 2025

I went with snow_depth because it appears that snowfall and snowdepth are actually two different measurements

I think that's correct but it would be great to confirm.

poa_global_* instead of poa_irrad_*?

I agree with poa_global_*. Here "global" means both beam and diffuse.

Let's leave the 7xxx data out of the map if we can't find clarification on the quantities.

@chzapo
Copy link
Author

chzapo commented Jan 23, 2025

I am currently in contact with someone at University of Oregon to discuss the 7xxx data descriptions. He said he did not have time this week to talk, but would get back to me next week when things had lightened up a bit. Hopefully we can get a satisfactory answer on what exactly each measurement there represents.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants