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.

Sign up for GitHub

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

Jump to bottom

Unfied forcing rebase #406

Merged
merged 36 commits into from
May 6, 2022
Merged

Unfied forcing rebase #406

merged 36 commits into from
May 6, 2022

Conversation

donaldwj
Copy link
Contributor

@donaldwj donaldwj commented Apr 6, 2022

[This pull request updates all forcing source to use the new template based DataProvider API]

Additions

  • DataProvider Template base class

  • DataSelectorClasses to use with DataProvider

  • GenericDataProvider to act as a replacement for ForcingProvider

Removals

The ForcingProvider class should no longer be in any framework code. Eventually the file should be removed.

Changes

Testing

  1. All built-in unit test pass with the exception of those that fail on master.

Screenshots

Notes

Todos

Checklist

  • [X PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • [~] Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist (automated report can be put here)

Target Environment support

  • Linux

hellkite500
hellkite500 reviewed Apr 7, 2022
View reviewed changes
include/forcing/Forcing.h Show resolved Hide resolved
include/forcing/Forcing.h Show resolved Hide resolved
include/forcing/DataProvider.hpp Show resolved Hide resolved
include/forcing/Forcing.h Show resolved Hide resolved
include/forcing/CsvPerFeatureForcingProvider.hpp Show resolved Hide resolved
include/forcing/NetCDFPerFeatureDataProvider.hpp Show resolved Hide resolved
include/forcing/NetCDFPerFeatureDataProvider.hpp Outdated Show resolved Hide resolved
include/forcing/NetCDFPerFeatureDataProvider.hpp Outdated Show resolved Hide resolved
include/forcing/NetCDFPerFeatureDataProvider.hpp Outdated Show resolved Hide resolved
include/forcing/NetCDFPerFeatureDataProvider.hpp Show resolved Hide resolved
@donaldwj donaldwj force-pushed the unfied_forcing_rebase branch from 638e908 to 45d09ba Compare April 13, 2022 18:30
mattw-nws
mattw-nws requested changes Apr 26, 2022
View reviewed changes
include/realizations/catchment/Bmi_Multi_Formulation.hpp Outdated Show resolved Hide resolved
include/forcing/CsvPerFeatureForcingProvider.hpp Outdated Show resolved Hide resolved
include/forcing/WrappedForcingProvider.hpp Outdated Show resolved Hide resolved
include/realizations/catchment/Bmi_Module_Formulation.hpp Outdated Show resolved Hide resolved
test/forcing/TrivialForcingProvider.hpp Outdated Show resolved Hide resolved
test/forcing/TrivialForcingProvider.hpp Outdated Show resolved Hide resolved
mattw-nws
mattw-nws requested changes Apr 28, 2022
View reviewed changes
include/realizations/catchment/Bmi_Multi_Formulation.hpp Show resolved Hide resolved
include/realizations/catchment/Bmi_Module_Formulation.hpp Show resolved Hide resolved
include/realizations/catchment/Bmi_Module_Formulation.hpp Show resolved Hide resolved
include/realizations/catchment/Bmi_Module_Formulation.hpp Outdated Show resolved Hide resolved
include/realizations/catchment/Bmi_Module_Formulation.hpp Outdated Show resolved Hide resolved
src/realizations/catchment/Simple_Lumped_Model_Realization.cpp Show resolved Hide resolved
include/forcing/CsvPerFeatureForcingProvider.hpp Show resolved Hide resolved
test/forcing/CsvPerFeatureForcingProvider_Test.cpp Show resolved Hide resolved
include/realizations/catchment/Bmi_Module_Formulation.hpp Show resolved Hide resolved
test/forcing/OptionalWrappedProvider_Test.cpp Show resolved Hide resolved
donaldwj added 22 commits May 5, 2022 14:16
@donaldwj
Add inital Data Provider template.
099a0a1
@donaldwj
Updated commit to add documentation of new parameter.
b5ebf89
@donaldwj
Add missing include to DataProvider
e085d12
@donaldwj
Add Netcdf Data Provider for per catchment data streams.
b6afae9
@donaldwj
Fix typos in comments.
e7a327c
@donaldwj
Add file for Netcdf forcing unit tests.
e2cd1ce
@donaldwj
Restoring conversion file lost in bad git update.
feb5447
@donaldwj
Bug fixes to restored file.
0e6334b
@donaldwj
Make cmake build the netcdf tests.
c9f4517
@donaldwj
Added code to build extrac ids from netcdf file and create the map of…
de53368 
… id->offset. Added stand alone test for netcdf forcing.
@donaldwj
Inital implementation of reading data from netcdf. Testing for cases …
fee022b 
…where either one or two underling data values are accessed for the requested value.
@donaldwj
Changed how string allocated by netcdf library are released.
8eba345
@donaldwj
Added UDUnits conversion.
370c89a
@donaldwj
fix bug in general case data scaling.
6f64c21
@donaldwj
bug fixes
18288c7
@donaldwj
Changes based on review.
d73ff87
@donaldwj
Clarify a comment on scale_factor for mean sampling.
ec4796f
@donaldwj
Update scale_factor for edge case of partial read of two cells with l…
426b85d 
…ess than one time peroid duration.
@donaldwj
Added request features loging suport and validation of time intervals.
f7bb229
@donaldwj
Added check for incorrect variable name in data request.
2dd184c
@donaldwj
Inital implementation of Data Selectors.
32d46c6
@donaldwj
Update NetCDFPerFeatureDataProvider to use new DataProvider API.
42297cb
@donaldwj donaldwj force-pushed the unfied_forcing_rebase branch from b9cbf42 to e2563f5 Compare May 5, 2022 14:17
@mattw-nws mattw-nws self-requested a review May 5, 2022 14:47
mattw-nws
mattw-nws reviewed May 5, 2022
View reviewed changes
Copy link
Contributor

@mattw-nws mattw-nws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to get the diffs to update...

@mattw-nws mattw-nws self-requested a review May 5, 2022 15:05
@mattw-nws mattw-nws merged commit 86d6e99 into NOAA-OWP:master May 6, 2022
mattw-nws added a commit to mattw-nws/ngen that referenced this pull request May 23, 2022
@mattw-nws
Fixes to merge with NOAA-OWP#406
e448605
hellkite500 pushed a commit to hellkite500/ngen that referenced this pull request May 26, 2022
@mattw-nws @hellkite500
Fixes to merge with NOAA-OWP#406
37e4416
mattw-nws added a commit that referenced this pull request Jul 11, 2022
@mattw-nws
Fixes to merge with #406
b2e60b5
@mattw-nws mattw-nws mentioned this pull request Jan 13, 2023
Closed
@mattw-nws mattw-nws mentioned this pull request Jul 10, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hellkite500 hellkite500 hellkite500 left review comments

@mattw-nws mattw-nws mattw-nws approved these changes

@robertbartel robertbartel Awaiting requested review from robertbartel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@donaldwj @hellkite500 @mattw-nws @stcui007