Load resource data from a folder without subdirectories of the resource type

[elenya-grant]

Load resource data from a folder without subdirectories of the resource type

Before this PR, resource data files had to exist in a folder with the following format: top_level_resource_dir/wind/ or top_level_resource_dir/solar. This PR allows for resource data to be loaded (but not saved to) from folders top_level_resource_dir/resource_filename.ext. This PR would resolve Issue #329

Section 1: Type of Contribution

• Feature Enhancement
  • Framework
  • New Model
  • Updated Model
  • Tools/Utilities
  • Other (please describe): added flexibility
• Bug Fix
• Documentation Update
• CI Changes
• Other (please describe):

Section 2: Draft PR Checklist

• Open draft PR
• Describe the feature that will be added
• [-] Fill out TODO list steps
• [-] Describe requested feedback from reviewers on draft PR
• [-] Complete Section 7: New Model Checklist (if applicable)

TODO:

N/A

Type of Reviewer Feedback Requested (on Draft PR)

Structural feedback:

Implementation feedback:

Other feedback:

Section 3: General PR Checklist

• PR description thoroughly describes the new feature, bug fix, etc.
• Added tests for new functionality or bug fixes
• Tests pass (If not, and this is expected, please elaborate in the Section 6: Test Results)
• Documentation
  • Docstrings are up-to-date
  • Related docs/ files are up-to-date, or added when necessary
  • Documentation has been rebuilt successfully
  • [-] Examples have been updated (if applicable)
• CHANGELOG.md has been updated to describe the changes made in this PR

Section 3: Related Issues

#329

Section 4: Impacted Areas of the Software

Section 4.1: New Files

None

Section 4.2: Modified Files

• h2integrate/resource/wind/test/test_nrel_developer_wtk_api.py
  • test_wind_resource_loaded_from_weather_dir: new test for added functionality/flexibility.
• h2integrate/resource/resource_base.py
  • ResourceBaseAPIModel.get_data(): updated to check if a pre-downloaded file exists in a top-level resource folder. If that file doesn't exist, a subdirectory named as the resource type will be created (as before) and the resource file will be downloaded there.

Section 5: Additional Supporting Information

Section 6: Test Results, if applicable

Section 7 (Optional): New Model Checklist

• Model Structure:
  • Follows established naming conventions outlined in docs/developer_guide/coding_guidelines.md
  • Used attrs class to define the Config to load in attributes for the model
    • If applicable: inherit from BaseConfig or CostModelBaseConfig
  • Added: initialize() method, setup() method, compute() method
    • If applicable: inherit from CostModelBaseClass
• Integration: Model has been properly integrated into H2Integrate
  • Added to supported_models.py
  • If a new commodity_type is added, update create_financial_model in h2integrate_model.py
• Tests: Unit tests have been added for the new model
  • Pytest-style unit tests
  • Unit tests are in a "test" folder within the folder a new model was added to
  • If applicable add integration tests
• Example: If applicable, a working example demonstrating the new model has been created
  • Input file comments
  • Run file comments
  • Example has been tested and runs successfully in test_all_examples.py
• Documentation:
  • Write docstrings using the Google style
  • Model added to the main models list in docs/user_guide/model_overview.md
    • Model documentation page added to the appropriate docs/ section
    • <model_name>.md is added to the _toc.yml

[genevievestarke]

I think this pull request looks good!
I do have a general comment on whether H2I gives a warning when the resource file doesn't exist so it downloads data from the API? I think it could help to have a warning if the user input a resource directory, but it wasn't loaded because the file didn't exist (if a warning is not already in there somewhere!).

[elenya-grant]

I think this pull request looks good! I do have a general comment on whether H2I gives a warning when the resource file doesn't exist so it downloads data from the API? I think it could help to have a warning if the user input a resource directory, but it wasn't loaded because the file didn't exist (if a warning is not already in there somewhere!).

There is not a warning at the moment, I am not sure if a warning would break an optimization or design sweep if it was downloading resource data. Perhaps the behavior of the resource models could be documented in a doc page instead? What are your thoughts?

[genevievestarke]

There is not a warning at the moment, I am not sure if a warning would break an optimization or design sweep if it was downloading resource data. Perhaps the behavior of the resource models could be documented in a doc page instead? What are your thoughts?

I've been thinking about this, and I think what I actually want is if the user provides a file name variable, it gives a warning that it couldn't find that file. I can see if we're doing a sweep or want to specify a folder for the resource data we wouldn't want that warning, I was just thinking that if you actually specify a resource file name, it would be nice to know if it was using that or downloading data.

[elenya-grant]

There is not a warning at the moment, I am not sure if a warning would break an optimization or design sweep if it was downloading resource data. Perhaps the behavior of the resource models could be documented in a doc page instead? What are your thoughts?

I've been thinking about this, and I think what I actually want is if the user provides a file name variable, it gives a warning that it couldn't find that file. I can see if we're doing a sweep or want to specify a folder for the resource data we wouldn't want that warning, I was just thinking that if you actually specify a resource file name, it would be nice to know if it was using that or downloading data.

I added in a UserWarning to resource_base.py (also required modifying the warnings filter in the electrolyzer models so that the warning is actually printed).

[johnjasa]

There is not a warning at the moment, I am not sure if a warning would break an optimization or design sweep if it was downloading resource data. Perhaps the behavior of the resource models could be documented in a doc page instead? What are your thoughts?

I've been thinking about this, and I think what I actually want is if the user provides a file name variable, it gives a warning that it couldn't find that file. I can see if we're doing a sweep or want to specify a folder for the resource data we wouldn't want that warning, I was just thinking that if you actually specify a resource file name, it would be nice to know if it was using that or downloading data.

I added in a UserWarning to resource_base.py (also required modifying the warnings filter in the electrolyzer models so that the warning is actually printed).

I like this conversation!

In the event that a user inputs a filename and it's not actually there, should we instead throw an Error (i.e. halt execution)? I would think (as a user) if I provided a filename, it would use that one. If it's not there, I'd maybe want to halt the run and know about it so I could fix it? I fear a warning may get lost in the terminal output or would lead to resource data being downloaded without the user appreciating it.

I'm good with the warning setup as-is, but more wanted to ask what behavior you would expect as a user.

[johnjasa]

Nice fix to a longstanding and frustrating behavior with our resource files! I'm glad to have this in, especially for when you and others do more large-scale studies.

I've added some ever-so-slightly-blocking comments that I'd like your take on, mostly about how we handle warnings. Let me know on Teams when you've commented and I'll take a quick look and we can go from there. Thanks!

[johnjasa]

For this and other filters you've added in this PR, should we do any larger changes to the way the problem is formulated to avoid these warnings? If you haven't yet, could you please capture this in an issue? In general, I'd prefer not to filter warnings because it might hide undesirable behavior, especially in the future.

[elenya-grant]

I modified existing filters so that the warning in the resource baseclass would actually be seen, I think some warnings were filtered out because of line in h2integrate/converters/hydrogen/pem_model/run_PEM_main.py. I added in the filtered warnings in the electrolyzer files to be more targeted to specific and expected warnings than the previous line which was just warnings.filterwarnings('ignore'). I can make an issue though.

[elenya-grant]

With respect to your larger question about error vs warning for the resource file not being found - I think a warning makes sense but I suppose that theres two reasons why a user may specify a filename:

1. To specify a file that already exists/has been downloaded to be used (this is the case where it'd make sense to throw an error if that file isn't found)
2. To specify the filename that you want resource data to be downloaded to (that doesn't follow the naming convention in the resource models). Aka - I may specify a filename as "tx_site_wind_data_2013.csv" and want the data downloaded for the input latitude/longitude to that file. In this case, it would not make sense to throw an error.

Given these two potential use-cases, I think a warning is a nice middle ground.

[johnjasa]

Understood on all fronts, thanks for the response!

Please do make an issue to remove the warning filters throughout the code, noting the areas you've already mentioned here.

Thank you for sharing those two use cases -- I didn't fully appreciate the capabilities or desired behavior of the second one. This makes sense!

[elenya-grant]

@johnjasa - I made issue #476 about filtering warnings!

[genevievestarke]

Looks good to me, I like the changes made!

[johnjasa]

Thanks Elenya!