Fix/floris

[jaredthomas68]

Correct floris tests

The tests were not passing locally for me for floris after the merging in of PRs this morning, adding this file from openmeteo solved the issue. Adding the file may be what is required, but it looks like the only difference between the new file and the old one is a utc vs local spec at the end. Perhaps my system is searching for the wrong time zone. Thoughts?

Changing the verify download flag in the openmeteo download call to True, also fixed the error

Section 1: Type of Contribution

• Feature Enhancement
  • Framework
  • New Model
  • Updated Model
  • Tools/Utilities
  • Other (please describe):
• 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:

• Step 1
• Step 2

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

Section 4: Impacted Areas of the Software

Section 4.1: New Files

• path/to/file.extension
  • method1: What and why something was changed in one sentence or less.

Section 4.2: Modified Files

• path/to/file.extension
  • method1: What and why something was changed in one sentence or less.

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

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[johnjasa]

Good from my end, waiting for Elenya to go through and see why the test passed CI but failed locally

[elenya-grant]

I had a lot of thoughts that I put in a single comment, but I think this is the simplest solution to this problem. Thanks for catching this!

[elenya-grant]

I think rather than introduce a new file, that we could update the inputs to some tests. The file "open-meteo-44.04N95.20W438m.csv" (already downloaded and in UTC timezone) is used to test if the openmeteo wind resource model works with files downloaded from the web. The file "resource_files/wind/44.04218_-95.19757_2023_openmeteo_archive_60min_local_tz.csv" is used to test the openmeteo wind resource with files downloaded from the openmeteo API using H2I. To get data downloaded in the "local" timezone, the timezone has to be specified in the plant config basically as anything that is non-zero (see the difference in the plant configs fixtures in h2integrate/resource/wind/test/test_openmeteo_wind_api.py).

I am not so concerned about the CI failing on Jared's computer but not on Github Actions - this is why PR #482 was introduced. I'm more concerned that the tests that should be using a file in resource_files is not finding that right file (which is likely because we need to update the timezone in these tests). An alternative to adding this file would be update/add the timezone parameter to the plant configs used for these tests:

1. add "timezone": -6 to plant_config_openmeteo fixture in h2integrate/converters/wind/test/test_floris_wind.py

plant_dict = {
        "plant_life": 30,
        "simulation": {"n_timesteps": 8760, "dt": 3600, "start_time": "01/01 00:30:00", "timezone": -6},
    }

2. add the timezone to examples/26_floris/plant_config.yaml. This would make the utility_wind_site use the resource file resource_files/wind/35.2018863_-101.945027_2012_wtk_v2_60min_local_tz.csv instead of resource_files/wind/35.2018863_-101.945027_2012_wtk_v2_60min_utc_tz.csv and the distributed_wind_site to use resource_files/wind/44.04218_-95.19757_2023_openmeteo_archive_60min_local_tz.csv instead of resource_files/wind/44.04218_-95.19757_2023_openmeteo_archive_60min_utc_tz.csv.

sites:
  distributed_wind_site: #name of the site for the distributed_wind_plant technology
    latitude: 44.04218
    longitude: -95.19757
    resources:
      wind_resource: #name of the resource model for this site
        resource_model: "OpenMeteoHistoricalWindResource"
        resource_parameters:
          resource_year: 2023
  utility_wind_site: #name of the site for the utility_wind_plant technology
    latitude: 35.2018863
    longitude: -101.945027
    resources:
      wind_resource: #name of the resource model for this site
        resource_model: "WTKNRELDeveloperAPIWindResource"
        resource_parameters:
          resource_year: 2012
plant:
  plant_life: 30
  simulation:
    n_timesteps: 8760
    dt: 3600
    timezone: -6

Except that this update would likely cause some subtests to fail because the wind generation profile would be different.

Aka - take all of the above as further explanation about what's going on in the resource models. I think that adding this file is the simplest solution to fix Jared's problem about these tests failing locally while also preventing the github actions from repeatedly calling the openmeteo wind API.

[jaredthomas68]

Adding the timezone worked. I personally prefer that fix and it did not break any other tests. I have push a new fix with the two line changes suggested by @elenya-grant. Thanks!