1011 formula request add table 6.6 from eurocode 1993 1 1 - #1041
1011 formula request add table 6.6 from eurocode 1993 1 1#1041GerjanDorgelo wants to merge 8 commits into
Conversation
…to EN 1993-1-1:2005
…ng usage instructions for moment distribution analysis
|
Thank you so much for contributing to Blueprints! Now that you've created your pull request, please don't go away; take a look at the bottom of this page for the automated checks that should already be running. If they pass, great! If not, please click on 'Details' and see if you can fix the problem they've identified. A maintainer should be along shortly to review your pull request and help get it added! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughImplements EN 1993-1-1:2005 Table 6.6 k_c calculation with lookup and ψ-based formulas, normalized moment-shape fitting using scipy, automatic distribution inference, LaTeX output, and comprehensive tests. ChangesEN 1993-1-1 Table 6.6 k_c Correction Factor
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1041 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 567 568 +1
Lines 15859 15979 +120
==========================================
+ Hits 15859 15979 +120 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
tests/codes/eurocode/en_1993_1_1_2005/chapter_6_ultimate_limit_state/test_table_6_6.py (1)
213-222: 💤 Low valueBrittle assertion path for all-zero input.
For
y_data = [0,0,0,0,0,0],_calculate_r2returns 0.0 for every fit (sincess_tot == 0andss_res != 0), so all entries inall_resultstie atr2_score=0.0. The test only passes becausemax(...)with ties returns the first item by insertion order ("Line 1 (Constant)" → CONSTANT → k_c=1.0). This is correct in CPython 3.7+ but depends on a subtle implementation property ofmaxoverdict.items(). Consider documenting or short-circuiting the all-zero / no-variance case explicitly in the implementation, e.g. returning 1.0 whennp.max(np.abs(y_data)) == 0.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/codes/eurocode/en_1993_1_1_2005/chapter_6_ultimate_limit_state/test_table_6_6.py` around lines 213 - 222, The test is brittle because interpretation_of_moment_distribution_for_kc ties when y_data has zero variance; to fix, add an explicit early-return in Table6Dot6CorrectionFactorKc.interpretation_of_moment_distribution_for_kc that detects the zero-moment/no-variance case (e.g. if np.max(np.abs(y_data)) == 0 or ss_tot == 0) and returns 1.0 immediately; this prevents relying on _calculate_r2/all_results tie-breaking and keeps existing fit logic for normal inputs.blueprints/codes/eurocode/en_1993_1_1_2005/chapter_6_ultimate_limit_state/table_6_6.py (1)
431-437: 💤 Low valueLinear-case slope extraction is fragile.
all_results.get("Line 2 (Linear)", {}).get("parameters", [None])[0]will raiseIndexErrorifparametersis ever stored as an empty array/sequence (e.g. an exception duringcurve_fitleft a partial entry). Today this should not happen because every successful insertion includes the fittedpopt, but the chain reads as defensive code that isn't fully defensive. Consider an explicit guard.📝 Proposed refactor
# Calculate k_c based on the distribution type if distribution_type == MomentDistributionType.LINEAR: - line_2_slope = all_results.get("Line 2 (Linear)", {}).get("parameters", [None])[0] - line_2_psi = 1 - 2 * line_2_slope if line_2_slope is not None else None - k_c = 1 / (1.33 - 0.33 * line_2_psi) if line_2_psi is not None else 1.0 + line_2_params = all_results.get("Line 2 (Linear)", {}).get("parameters") + line_2_slope = line_2_params[0] if line_2_params is not None and len(line_2_params) > 0 else None + if line_2_slope is None: + k_c = 1.0 + else: + line_2_psi = 1 - 2 * line_2_slope + k_c = 1 / (1.33 - 0.33 * line_2_psi)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@blueprints/codes/eurocode/en_1993_1_1_2005/chapter_6_ultimate_limit_state/table_6_6.py` around lines 431 - 437, The extraction of the linear slope is not defensive against an empty "parameters" sequence and can raise IndexError; in the branch checking MomentDistributionType.LINEAR (symbols: distribution_type, MomentDistributionType.LINEAR, all_results key "Line 2 (Linear)"), explicitly fetch the parameters container and verify it's a non-empty sequence before reading index 0 (set line_2_slope to None if missing), then compute line_2_psi and k_c as before; this keeps the fallback k_c = 1.0 behavior when parameters are absent or malformed and avoids an IndexError when parameters == [] or None.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@blueprints/codes/eurocode/en_1993_1_1_2005/chapter_6_ultimate_limit_state/table_6_6.py`:
- Around line 357-361: The fallback return value for the function that computes
the correction factor k_c currently returns 0.0 but the docstring and other
degenerate branches (empty/single-point and non-LINEAR distribution_type)
indicate it should return 1.0; update the final fallback return in the k_c
calculation to return 1.0 instead of 0.0 so the behavior matches the docstring
and the existing degenerate-case returns.
- Around line 361-367: The docstring example calls a non-existent method
interpret_moment_distribution_for_kc causing AttributeError; update the example
to call the actual classmethod interpretation_of_moment_distribution_for_kc on
Table6Dot6CorrectionFactorKc (i.e., replace interpret_moment_distribution_for_kc
with interpretation_of_moment_distribution_for_kc in the example) so the pasted
example runs correctly and reflects the real API.
- Around line 285-294: The _normalize function may divide by zero when x_max ==
x_min; add a guard mirroring the y_data check: compute x_min and x_max as
before, then if x_max != x_min perform x_data = (x_data - x_min) / (x_max -
x_min), else set x_data to np.zeros_like(x_data) (or (x_data - x_min) which
yields zeros) to avoid NaN/inf; keep the existing y_data guard unchanged and
return the normalized arrays.
In
`@tests/codes/eurocode/en_1993_1_1_2005/chapter_6_ultimate_limit_state/test_table_6_6.py`:
- Around line 137-185: The test docstrings incorrectly state that mirroring is
being tested while Table6Dot6CorrectionFactorKc._normalize only rescales x-range
and y-amplitude; update the docstrings in the four tests
(test_normalizing_data_mid_positive_right_negative,
test_normalizing_data_mid_negative_left_positive,
test_normalizing_data_mid_negative_left_negative,
test_normalizing_data_mid_negative_right_postive) to say they verify
rescaling/normalization (not mirroring) and fix the typo "postive" → "positive"
in the last test name/docstring so the intent matches the behavior of
_normalize.
- Around line 380-389: The interpretation_of_moment_distribution_for_kc
currently relies on x,y ordering and fails for non-monotonic x_data; after
calling Table6Dot6CorrectionFactorKc._normalize, pair the normalized x and y
values into (x,y) tuples, sort them by x, then unpack back to x_data,y_data
before calling piecewise fitters such as _line_6_simply_supported_f;
alternatively, add an explicit monotonicity check at the start of
interpretation_of_moment_distribution_for_kc and raise a clear error if x is not
strictly increasing — implement the sorting approach to preserve behavior for
arbitrary input order.
---
Nitpick comments:
In
`@blueprints/codes/eurocode/en_1993_1_1_2005/chapter_6_ultimate_limit_state/table_6_6.py`:
- Around line 431-437: The extraction of the linear slope is not defensive
against an empty "parameters" sequence and can raise IndexError; in the branch
checking MomentDistributionType.LINEAR (symbols: distribution_type,
MomentDistributionType.LINEAR, all_results key "Line 2 (Linear)"), explicitly
fetch the parameters container and verify it's a non-empty sequence before
reading index 0 (set line_2_slope to None if missing), then compute line_2_psi
and k_c as before; this keeps the fallback k_c = 1.0 behavior when parameters
are absent or malformed and avoids an IndexError when parameters == [] or None.
In
`@tests/codes/eurocode/en_1993_1_1_2005/chapter_6_ultimate_limit_state/test_table_6_6.py`:
- Around line 213-222: The test is brittle because
interpretation_of_moment_distribution_for_kc ties when y_data has zero variance;
to fix, add an explicit early-return in
Table6Dot6CorrectionFactorKc.interpretation_of_moment_distribution_for_kc that
detects the zero-moment/no-variance case (e.g. if np.max(np.abs(y_data)) == 0 or
ss_tot == 0) and returns 1.0 immediately; this prevents relying on
_calculate_r2/all_results tie-breaking and keeps existing fit logic for normal
inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e23e0fd3-e75c-4435-901e-7eb2d3564af6
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
blueprints/codes/eurocode/en_1993_1_1_2005/chapter_6_ultimate_limit_state/table_6_6.pypyproject.tomltests/codes/eurocode/en_1993_1_1_2005/chapter_6_ultimate_limit_state/test_table_6_6.py
|
@GerjanDorgelo check coderabbit comments |
egarciamendez
left a comment
There was a problem hiding this comment.
check coderabbit comments
|
@egarciamendez oh. good call! ez fixes. |
Description
Add table 6.6 from eurocode 1993 1 1
Fixes #1011
Type of change
Please delete options that are not relevant.
Checklist:
Summary by CodeRabbit