Skip to content

consistent use xr.DataArray and xr.Dataset arguments #248

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.

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

Merged
merged 5 commits into from
Nov 3, 2023

Conversation

jtgrasb
Copy link
Collaborator

@jtgrasb jtgrasb commented Jun 27, 2023

Description

This pull request makes the exc_coeff to be consistently a DataArray throughout the multiple functions that use it. I also briefly checked some of the other docstrings to confirm inputs and outputs are accurately recorded.

Closes #183

Type of PR

  • Bug fix
  • New feature
  • Documentation
  • Other: (specify)

Checklist for PR

Additional details

This is in response to issue #183. @akeow I don't know what caused the initial issue so please let me know whether this solves the issue.

@coveralls
Copy link

coveralls commented Jun 28, 2023

Pull Request Test Coverage Report for Build 6747692411

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 91.897%

Totals Coverage Status
Change from base Build 6735628848: 0.0%
Covered Lines: 2359
Relevant Lines: 2567

💛 - Coveralls

@jtgrasb
Copy link
Collaborator Author

jtgrasb commented Jun 30, 2023

Solve docstring should be fixed now @cmichelenstrofer

@rgcoe
Copy link
Collaborator

rgcoe commented Oct 6, 2023

@jtgrasb - I think this may also need to be updated for the static methods for creating wec objects. For example

@staticmethod
def from_impedance(
freqs: ArrayLike,
impedance: ArrayLike,
exc_coeff: ArrayLike,
hydrostatic_stiffness: ndarray,
f_add: Optional[TIForceDict] = None,
constraints: Optional[Iterable[Mapping]] = None,
min_damping: Optional[float] = _default_min_damping,
) -> TWEC:
"""Create a WEC object from the intrinsic impedance and
excitation coefficients.
The intrinsic (mechanical) impedance :math:`Z(ω)` linearly
relates excitation forces :math:`F(ω)` to WEC velocity
:math:`U(ω)` as :math:`ZU=F`.
Using linear hydrodynamic coefficients, e.g. from a BEM code
like Capytaine, the impedance is given as
:math:`Z(ω) = (m+A(ω))*iω + B(ω) + B_f + K/(iω)`.
The impedance can also be obtained experimentally.
Note that the impedance is not defined at :math:`ω=0`.
Parameters
----------
freqs
Frequency vector [:math:`Hz`] not including the zero frequency,
:python:`freqs = [f1, 2*f1, ..., nfreq*f1]`.
impedance
Complex impedance of size :python:`(ndof, ndof, nfreq)`.
exc_coeff
Complex excitation transfer function of size
:python:`(ndof, nfreq)`.
hydrostatic_stiffness
Linear hydrostatic restoring coefficient of size
:python:`(ndof, ndof)`.
f_add
Dictionary with entries :python:`{'force_name': fun}`, where
:python:`fun` has a signature
:python:`def fun(wec, x_wec, x_opt, waves):`, and returns
forces in the time-domain of size
:python:`(2*nfreq, ndof)`.
constraints
List of constraints, see documentation for
:py:func:`scipy.optimize.minimize` for description and
options of constraints dictionaries.
If :python:`None`: empty list :python:`[]`.
min_damping
Minimum damping level to ensure a stable system.
See :py:func:`wecopttool.check_impedance` for
more details.

We should get consistent with this, or (ideally) program in a way that is agnostic to whether the users passes an xr.dataarray or np.ndarray

@jtgrasb jtgrasb changed the title Change exc_coeff to a dataset Make exc_coeff consistently an Arraylike input Oct 12, 2023
@jtgrasb
Copy link
Collaborator Author

jtgrasb commented Oct 12, 2023

@ryancoe I changed the title of this PR as it was backwards previously. The PR makes exc_coeff an ArrayLike input wherever it is used. The ArrayLike specification is agnostic to whether the users passes an xr.dataarray or np.ndarray. I think what you're suggesting is to be consistent for other variables such as the hydrostatic_stiffness? If so, I can amend this to make sure that and other variables are all agnostic to array input type.

@rgcoe
Copy link
Collaborator

rgcoe commented Nov 2, 2023

Per discussion today: we like indexing by name, so require the arguments to be xr.DataArrays, etc. After that, merge.

@rgcoe rgcoe changed the title Make exc_coeff consistently an Arraylike input exc_coeff uses xr.DataArray arguments Nov 2, 2023
@rgcoe rgcoe changed the title exc_coeff uses xr.DataArray arguments consistent use xr.DataArray and xr.Dataset arguments Nov 2, 2023
@jtgrasb jtgrasb marked this pull request as ready for review November 3, 2023 16:16
@jtgrasb jtgrasb merged commit 637cfa5 into sandialabs:main Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

exc_coeff needs to be an xarray.Dataset
3 participants