Convert power module to xarray #282
Conversation
There was a problem hiding this comment.
@akeeste thank you for putting this together and great job. In this review I basically want to
- Add type checks to the parameters this PR adds
- Minor clean up from previous authors
- Couple of minor questions
I still need to run through the example notebook tomorrow
| power = dc_power(voltage, current, to_pandas=False)['Gross'] | ||
| power.name = 'Power' | ||
| power = power.to_dataset() # force xr.DataArray to be consistently in xr.Dataset format |
There was a problem hiding this comment.
This forcing seems like a intermediate step needed prior to getting dc_power to return an xarray.
Is this still needed when we have to_pandas=False?
There was a problem hiding this comment.
When the Gross power variable is extracted from the results of dc_power, the xr.Dataset automatically becomes an xr.DataArray since there's just one variable. I originally added this conversion to a xr.Dataset to prevent that type change.
There shouldn't be any issues with returning a single variable array like pd.Series or xr.DataArray, but the previous function and most of the current ones I'm working with return pd.DataFrame and xr.Dataset
mhkit/tests/power/test_power.py
Outdated
| hsg = power.quality.harmonic_subgroups(harmonics, self.frequency) | ||
| TCHD = power.quality.total_harmonic_current_distortion( | ||
| hsg) # had to just put a random rated current in here |
There was a problem hiding this comment.
Let's remove all instances of this comment while we are here and document somewhere in set up class or something is its really needed:
# had to just put a random rated current in here
There was a problem hiding this comment.
Good call. Also i actually just removed the rated current input all together, because it was not being used in anyway. I'm wondering if we're missing a scaling or something for THCD that would use the rated current
There was a problem hiding this comment.
Reviewing the definition of THCD (not with the marine energy IEC standard), I don't see a rated current used. From what I can tell, THCD is the ratio (in percent) between the energy of all harmonics and the energy of the dominant frequency and doesn't require a rated current.
mhkit/tests/power/test_power.py
Outdated
| self.assertEqual(P.sum()['Gross'], 584) | ||
| P = power.characteristics.dc_power(voltage['V1'], current['A1']) | ||
| self.assertEqual(P.sum()['Gross'], 70) |
There was a problem hiding this comment.
It looks like you changed this from recalculating the values to hardcoding them.
Are we getting the same values here as before or could you just comment on this change in general?
There was a problem hiding this comment.
These are exactly the same values that the test should give, just hard coded in. It's been a few weeks, I think I was having issues duplicating the recalculation method easily across pandas and xarray because of their different formats. Looking at it again, I think a better solution is to recalculate the power values, but with self.current_data and self.voltage_data. That would eliminate any differences between xarray/pandas and be easily repeatable across both tests
|
@ssolson Thanks for the review. I went through your comments and resolved them, which were primarily on adding parameter validation and doc strings where missing. I reverted the test to recalculate the answer as described above, and double-checked that the power notebook runs. I think the only outstanding item is to ensure that the THCD calculation is not missing anything. |
# MHKiT v0.8.0 We're excited to announce the release of MHKiT v0.8.0, which brings a host of new features, enhancements, and bug fixes across various modules, ensuring compatibility with Python 3.10 and 3.11, and introducing full xarray support for more flexible data handling. Significant updates in the Wave and DOLfYN modules improve functionality and extend capabilities. ## Python 3.10 & 3.11 Support MHKiT now supports python 3.10 and 3.11. Support for 3.12 will follow in the next minor update. - #240 ## Wave Module ### Enhancements: **Automatic Threshold Calculation for Peaks-Over-Threshold**: We've introduced a new feature that automatically calculates the "best" threshold for identifying significant wave events. This method, originally developed by Neary, V. S., et al. in their 2020 study, has now been translated from Matlab to Python, enhancing our existing peaks-over-threshold functionality. **Wave Heights Analysis**: A new function, `wave_heights`, has been added to extract the heights of individual waves from a time series. This function uses zero up-crossing analysis to accurately measure wave heights, improving upon our previous methods which only provided the maximum value between up-crossings. **Enhanced Zero Crossing Analysis**: Building on the above, the zero crossing code previously embedded in `global_peaks` has been isolated into a helper function. This modular approach not only refines the codebase but also supports new functionalities such as calculating wave heights, zero crossing periods, and identifying crests. ### Bug Fixes: **Contour Sampling Error in Wave Contours**: A bug identified in `mhkit.wave.contours.samples_contour` has been resolved. The issue occurred when period samples defined using the maximum period resulted in values outside the interpolation range of the contour data. This was corrected by ensuring that all sampling points are within the interpolation range and adjusting the contour data selection process accordingly. - #268 - #252 - #278 ## Xarray Support MHKiT functions now fully support the use of xarray for passing and returning data. - #279 - #282 - #285 - #302 - #310 ## DOLfYN Thanks to the many user contributions and users of MHKiT the DOLFYN module include a significant number of enhancements and bug fixes. ### Enhancements: **Altimeter Support**: Enhanced the Nortek Signature Reader to add capability for reading ADCP dual profile configurations. **Data Handling Improvements**: Introduced logic to skip messy header data that can accumulate during measurements collected via Nortek software on PCs and Macs. **Instrument Noise Subtraction**: Added a function to subtract instrument noise from turbulence intensity estimation using RMS calculations, providing results that differ by approximately 1% from the existing standard deviation-based "I" property. **Improved File Handling**: Updates for RDI files to handle changing "number of cells" and variable "cell sizes," which are now bin-averaged into the largest cell size. ### Bug Fixes: **Power Spectra Calculation**: Fixed a bug where a given noise value was not being subtracted from the power spectra, and noise was inadvertently added as an input to dissipation rate calculations. **Improved Header Handling**: Allowed RDI reader to skip junk headers effectively. **Nortek Reader C Types Update**: Adjusted C types in the Nortek reader to handle below-zero water temperatures and to allow pitch and roll values to go negative. - #280 - #289 - #290 - #292 - #293 - #294 - #299 ## River & Tidal: D3D Added limits to `variable_interpolation` and added 3 array input capability to `create_points` - #271 ## Developer Experience ### Black formatting Black formatting is now enforced on all MHKiT files. This ensures consistent formatting across the MHKiT package. - #281 ### Linting & Type Hints MHKiT is in the process of enforcing pylint and adding type hints to all functions. Currently this has been achieved and is enforced in the Loads and Power modules. - #288 - #296 ### CI/CD This release introduces significant reduction in testing time for development. This is achieved by reducing the number of tests for pulls against the develop branch and only running hindcast test when changes are made to it. A bug in the hindcast CI was fixed which only ran on changes to the hindcast tests instead of the hindcast module. Additionally the wave and wind hindcast needed to be separated in 2 jobs due to the excessive time taken to run a wind cache. This created a number of follow on PRs around solidifying the logic of these job. A special case for Python 3.8, pip, and Mac OS was added to use homebrew to install NetCDF and HDF5 to get tests to pass. - #241 - #270 - #306 - #311 - #317 - #318 - #319 - #320 - #324 ### Clean Up MHKiT fixed an implementation error where functions used assert instead of built in errors for type and value checking. Additionally these PRs removed unused files, fixed typos, and created an argument which allows users to run CDIP API calls silently. - #276 - #272 - #273
power.quality
power.characteristics
TODO