Repository navigation
fix: lump HydroDyn filled-member ballast into deck-built floating platforms - #143
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f6da2a23c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| i_mat = _platform_inertia_matrix(ptfm) | ||
| # Filled-member ballast, as in from_elastodyn_with_mooring. | ||
| if hydrodyn_dat is not None: | ||
| ptfm, i_mat = _add_filled_ballast(ptfm, i_mat, hydrodyn_dat) |
There was a problem hiding this comment.
rho when resolving default ballast density
When from_windio_floating falls back to WindIO hydro (for example, a supplied HydroDyn deck has PotMod=0) and the caller passes a non-default rho, the hydrostatic/added-mass calculations use that value, but this call resolves FillDens=DEFAULT to 1025 kg/m³ instead. A deck containing filled members therefore gets mutually inconsistent platform mass, inertia, and restoring terms; forward rho through _add_filled_ballast to read_filled_ballast(..., water_density=rho) on this path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a7b4ed5. from_windio_floating now passes its rho through _add_filled_ballast, so DEFAULT fill density matches the density the hydrostatics, added mass and mooring use on that path. from_elastodyn_with_mooring keeps the deck's own WtrDens. Covered by test_water_density_resolves_default_fill.
…last The OC4 DeepCwind sample now carries its ballasted platform and positive roll and pitch restoring; the IEA-15, IEA-22 and UPSCALE semis pick up the weight term in hydro_K. A default-run test keeps every bundled floater statically stable with all six rigid-body modes labelled.
4f6da2a to
a7b4ed5
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7b4ed5d1f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def _float(tok: str, what: str, path: pathlib.Path) -> float: | ||
| try: | ||
| return _parse_fortran_float(tok) |
There was a problem hiding this comment.
When a HydroDyn numeric field contains NaN or Inf (for example, an explicit FillDens or a joint coordinate), _parse_fortran_float accepts it and the later range checks do not reliably reject it because comparisons with NaN are false. The quadrature then returns non-finite ballast properties, which _add_filled_ballast propagates into the platform mass and inertia matrices instead of reporting the malformed deck; validate finiteness here before returning the parsed value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The shared _parse_fortran_float already raised on nan / inf, so these were rejected as malformed rather than propagated. The guarantee shouldn't rest on that helper, though, so ab0f561 checks finiteness directly in the ballast reader. New tests cover nan / inf in FillDens, FillFSLoc, a joint coordinate and PropD.
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Follow-up to #142, which left one case open. The OC4 DeepCwind semi keeps its water ballast in HydroDyn filled-member groups rather than in ElastoDyn's
PtfmMass, so its roll and pitch restoring stayed negative. This PR is stacked on #142 and should be retargeted tomasteronce #142 merges.What HydroDyn does with fill
Morison.f90treats a closed fill group as body-fixed ballast. It precomputes each element's fill mass, centroid and inertia (FloodedBallastPartSegmentCyl), and puts the internal pressure datum at the ballast's instantaneous high point. The net load is therefore the weight of a rigid mass at the fill centroid, with no free-surface correction. Groups open to the sea only arise for members buried in the seabed, which a floater doesn't have.Change
pybmodes.io._hydrodyn_ballast.read_filled_ballastreads the joints, cylindrical property sets, members and fill groups of a HydroDyn deck.FillFSLoc, following HydroDyn's own full / partial / empty logic, including a surface exactly at a horizontal member counting as full.NPropSetstable are accepted.FillDens = DEFAULTfalls back to the water density.NotImplementedErrorrather than being skipped.Tower.from_elastodyn_with_mooringand the deck tier ofTower.from_windio_floatinglump the ballast intomass_pform,cm_pformandi_matrixbefore the weight restoring is formed, using parallel-axis transfer to the combined CM.hydro_K, which they didn't carry yet.OC4 DeepCwind (r-test decks)
After the fix, in Hz: surge/sway 0.0088, heave 0.0579, roll/pitch 0.0390, yaw 0.0125. The combined roll/pitch inertia comes out 4 % above the published 6.827e9 kg·m². That traces to the steel inertias in the r-test ElastoDyn deck, not to the fill geometry.
Validation
tests/test_hydrodyn_ballast.pytests/test_floating_samples_spectra.py::test_floating_sample_is_statically_stable(default run) keeps every bundled floater free of the negative-restoring warning.VALIDATION.mdrows and the Robertson (2014) citation added;scripts/audit_validation_claims.pyis clean.ruffandmypyclean.