Adding deep water approximations #95

b0ndman · 2021-02-19T20:27:28Z

Deep water approximations for wave celerity (so sinh doesn't blow up to infinity) using new functions wave_length() and depth_regime(). wave_length input is wave number array, output is array of wavelengths indexed by frequency. depth_regime inputs are wavelength array, water column depth, and optional defining ratio of water depth/wavelength.

Deep water approximation for energy_flux also implemented using IEC 62600-100 eq. 8, resolves TODO in that function

ssolson · 2021-02-19T21:48:02Z

@b0ndman thank you for your contribution to MHKiT this looks like some really great functionality! I will be out of the office and unable to perform a formal review until later this month. The only thing currently missing is the tests for the new functions and possibly adding a test for deep=True in the current wave celerity test. If you are open to writing a couple of quick tests I could throw them into the test modules or we can work on them when I get back.

@rpauly18 speaking of tests it looks like NREL-rex failed the CI test on this PR build. If you get a moment could you check that out?

Thanks again @b0ndman great stuff!

rpauly18 · 2021-02-19T21:52:05Z

@ssolson yes, I have seen that and have started looking into it.

@b0ndman thanks for the contribution! I am also here to help with developing tests, just let us know if you are open to working on them.

…pull95

b0ndman · 2021-03-09T00:58:21Z

Apologies for kinda disappearing after doing the pull request (I'm on a bit of a tight work schedule, with the work on this stemming from that). I am open to helping out, at a limited capacity, and have noticed the NREL-rex failing also.

rpauly18 · 2021-03-09T18:59:13Z

@b0ndman thanks for offering to help. I have added a couple of tests for the depth_regime and wave_length functions, but if you have time and ability to review these tests and possibly add one for the deep water flag in energy flux it would be much appreciated. I am actively working on the NREL-rex issues that are being triggered by too many requests to the WPTO hindcast data server.

…pull95

ssolson

Hey, Rebecca apologizes for the delay of this review. Overall I had a couple of questions before we proceed. We can also schedule a chat to talk them over. Were you able to push directly to bondman's master branch to make changes?

ssolson · 2021-05-07T16:27:27Z

.gitignore

+# scratch file
+scratch.py
+scratch_data.txt


I don't think we want to add these here right?

removed in bc2720f

ssolson · 2021-05-07T16:29:54Z

mhkit/wave/resource.py

@@ -575,31 +577,43 @@ def energy_flux(S, h, rho=1025, g=9.80665):
    # TODO: Add deep water flag


We should remove this now.

Removed in ac13e8e. Also switched logic for better readability.

ssolson · 2021-05-07T16:31:41Z

mhkit/wave/resource.py

+        # Eq 8 in IEC 62600-100, deep water simpilification
+        Te = energy_period(S)
+        Hm0 = significant_wave_height(S)
+        # print(Hm_zero)


ssolson · 2021-05-07T19:36:50Z

mhkit/wave/resource.py

+
+    Returns
+    ---------
+    l: pandas Dataframe


Rebecca right now the terminology page lists wave length as $\lambda$. I can see calling wave length 'l' here as acceptable. Do you think we should spell it out though (calling it lambda) to avoid confusion or be consistent?

Variable can't be lamda because it's a built-in function. I will stick with "l" I guess.

ssolson · 2021-05-10T15:03:27Z

mhkit/wave/resource.py

+    else:
+        # check the depth for each of the frequencies using depth_regime()
+        # get wavelength
+        wl = wave_length(k)


This gets at my other comment but wave length should either be l to be consistent with the other parts of the code as written or lambda to be consistent with the terminology page.

ssolson · 2021-05-10T16:56:07Z

mhkit/wave/resource.py

+        by frequency
+    '''
+
+    assert isinstance(l, pd.DataFrame), "l must be of type pandas.DataFrame"


I think this function should work on numpy arrays too? We don't have to include it but is there a reason why we would only work on passed Dataframes and return a frequency indexed DataFrame?

ssolson · 2021-05-10T16:59:51Z

mhkit/wave/resource.py

+    l: pandas DataFrame
+        wavelength [m]


Dateframe indexed by anything? I think this should say indexed by frequency because dep_reg inherits its index from the wave length DataFrame here.

Do we have to do this? Would it be useful for this function to work on timeseries data? what about using it to check a particular wavelength like a float?

ssolson · 2021-05-10T17:15:00Z

mhkit/wave/resource.py

+    k: pandas Dataframe
+        Wave number [1/m] indexed by frequency


WHy does this have to be DataFrame indexed by frequency? It seems like it would be useful on time series data or even an int or float as well...thoughts?

Frequency is needed to calculate wave number, so when calculating it you get a k indexed by frequency. I suppose there could be use cases where a user would use a k not directly calculated from the MHKiT wave_number function that is a wave number for each time. So, for this particular function I suppose the index does not matter, I think he was just trying to be consistent with other functions that have k.

ssolson · 2021-05-10T17:17:34Z

mhkit/wave/resource.py

@@ -562,6 +562,8 @@ def energy_flux(S, h, rho=1025, g=9.80665):
        Spectral density [m^2/Hz] indexed by frequency [Hz]
    h: float
        Water depth [m]
+    deep: bool (optional)
+        Use deep water approximation


add default=False to docstring here

ssolson · 2021-05-10T19:29:59Z

mhkit/wave/resource.py

@@ -609,9 +623,11 @@ def wave_celerity(k, h, g=9.80665):
        Wave number [1/m] indexed by frequency [Hz]
    h: float
        Water depth [m]
+    depth_check: bool (optional)
+        If true check depth regime


Add 'default=False'

…ch one

ssolson · 2021-05-20T15:18:36Z

I have addressed all of my comments above. I think there are 2 issues outstanding after working with the code more.

Cannot change depth_ratio in depth_regime when calling from wave_celerity
No tests for the deep=True, and depth_check_=True

Currently, depth_regime allows a user to specify a depth ratio for the depth regime. However, when using wave celerity there is no way to change the default value of 2. To address this I suggest instead of only a boolean for the parameter deep we also allow ints/ floats. If an int/float is passed this is used instead of the default value.

For point 2 I will add tests to check the other half of the wave_celerity and energy_flux functions

rpauly18 · 2021-05-21T14:22:13Z

Good catch on point 1. I think it would be better to add ratio as an optional parameter in wave_celerity with the default=2. We can in the doc string indicate that for the parameter to have any effect check_depth must equal True. I will add this.

…ptions

… avg_0_crossing_period, and frequency_moment

ssolson · 2021-05-26T15:57:27Z

@rpauly18 I addressed the missing tests and coverage has been raised by 1%. I believe this is ready to go if you have a chance to look at it,

b0ndman added 3 commits February 15, 2021 16:18

progress

70391b2

weird pandas history fixing

919b003

adding optional ratio input in depth

9b69ba6

rpauly18 added 9 commits February 19, 2021 15:06

Merge branch 'master' of github.com:MHKiT-Code-Hub/mhkit-python into …

51f4c2c

…pull95

updating nrel-rex requirements

c8e0a40

Merge branch 'master' of github.com:MHKiT-Code-Hub/mhkit-python into …

b809bc8

…pull95

reverting package requirements for NREL-rex

7fe1c99

minor docstring updates and first test

fb15896

depth_regime test

4210598

fixing wave_length test

1de245d

consistant naming

dbad01b

Merge branch 'master' of github.com:MHKiT-Code-Hub/mhkit-python into …

013c6f0

…pull95

rpauly18 added 3 commits March 9, 2021 10:15

adding random wait before wpto hindcast calls

4f203a8

increasing wait time

4be4ecf

removing one test

49fa4db

rpauly18 added 5 commits March 9, 2021 14:35

re-adding test

5310ce0

merging hindcast test changes

077098b

working on tests

ab467a2

Merge branch 'master' of github.com:MHKiT-Code-Hub/mhkit-python into …

453b5a8

…pull95

adding energy_flux tests

f25cb76

rpauly18 requested a review from ssolson April 23, 2021 18:56

ssolson reviewed May 10, 2021

View reviewed changes

ssolson added 4 commits May 19, 2021 09:58

Remove scratch files from gitignore

bc2720f

remove TODO comment. Switch deep logic for better readability.

ac13e8e

Remove commented print statement

a466419

Generalize depth_regime to handle array-like data inputs, and test ea…

ab89813

…ch one

ssolson added 3 commits May 20, 2021 08:18

wave_length to take array-like input. Added its own test.

6803c94

Add default value to docstring

eb6a723

Uncomment the wave tests.

16fa5ea

ssolson added 2 commits May 20, 2021 13:06

In progress simplification of logic using mp masks

3847aa6

Simplified computation of wave celerity

8f42a84

rpauly18 and others added 6 commits May 21, 2021 08:34

adding ratio flag to wave_celerity

9a43d5f

add specific test for wave celrity. Test for deep and shollow water o…

2074401

…ptions

S dataframe or series

cf1fa98

Merge rpualy change

c7e71af

Add assert for ratio input

03450aa

Fix Bug in energy_flux deep. Allow for passing Series or DataFrame

b89ef95

rpauly18 mentioned this pull request May 24, 2021

Deep water approximations MHKiT-Software/MHKiT-MATLAB#60

Closed

ssolson added 4 commits May 25, 2021 16:21

Add test for energy_flux parameter deep=True

e120942

Allow passing of DataFrames or Series for energy_flux, energy_period,…

5ed2afc

… avg_0_crossing_period, and frequency_moment

Pass ratio from energy_flux to wave_celrity

e9abb66

Add default values to doc string

fe8e3a2

rpauly18 merged commit 82c3732 into MHKiT-Software:master Jun 4, 2021

rpauly18 mentioned this pull request Oct 27, 2021

Wave updates MHKiT-Software/MHKiT-MATLAB#70

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding deep water approximations #95

Adding deep water approximations #95

b0ndman commented Feb 19, 2021

ssolson commented Feb 19, 2021

rpauly18 commented Feb 19, 2021

b0ndman commented Mar 9, 2021

rpauly18 commented Mar 9, 2021

ssolson left a comment

ssolson May 7, 2021

ssolson May 19, 2021

ssolson May 7, 2021

ssolson May 19, 2021

ssolson May 7, 2021

ssolson May 7, 2021

ssolson May 19, 2021

ssolson May 10, 2021

ssolson May 10, 2021

ssolson May 10, 2021

ssolson May 10, 2021

rpauly18 May 21, 2021

ssolson May 10, 2021

ssolson May 10, 2021

ssolson commented May 20, 2021

rpauly18 commented May 21, 2021

ssolson commented May 26, 2021 •

edited

Loading

		@@ -575,31 +577,43 @@ def energy_flux(S, h, rho=1025, g=9.80665):
		# TODO: Add deep water flag

Adding deep water approximations #95

Adding deep water approximations #95

Conversation

b0ndman commented Feb 19, 2021

ssolson commented Feb 19, 2021

rpauly18 commented Feb 19, 2021

b0ndman commented Mar 9, 2021

rpauly18 commented Mar 9, 2021

ssolson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssolson commented May 20, 2021

rpauly18 commented May 21, 2021

ssolson commented May 26, 2021 • edited Loading

ssolson commented May 26, 2021 •

edited

Loading