Minor/fixes - #52
Merged
Merged
Conversation
- Bugfix tau sign - Updated the slab tensile criterion calculation to conditionally exclude low-density layers based on downward failure growth. - Improved docstrings for clarity on low-density threshold behavior. - Added tests to verify the correct application of the updated tensile criterion logic. - Adjusted comparison cases in tests for consistency with new logic.
- Bugfix tau sign - Updated the slab tensile criterion calculation to conditionally exclude low-density layers based on downward failure growth. - Improved docstrings for clarity on low-density threshold behavior. - Added tests to verify the correct application of the updated tensile criterion logic. - Adjusted comparison cases in tests for consistency with new logic.
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR refines weak-layer and slab mechanics computations through sign corrections to shear-stress formulas, introduces directional low-density propagation logic into slab tensile criterion evaluation, updates supporting docstrings, adds an energy-release-rate integrand validation test, and reorganizes project priorities in the TODO list. ChangesWeak-layer mechanics and slab tensile criterion refinements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts weak-layer shear stress sign/definition and refines how the slab tensile criterion treats low-density (“weak snow”) layers, with corresponding test updates and a few TODO additions.
Changes:
- Update weak-layer shear stress (
FieldQuantities.tau) formula/sign and align dependent unit tests. - Change the slab tensile criterion so low-density layers are excluded only after being “broken through” by downward failure growth; update config docs and add/adjust regression tests.
- Add a small Analyzer regression test around non-negative ERR integrands.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| TODO.md | Adds a few new major/minor TODO items. |
| tests/core/test_field_quantities.py | Updates expected weak-layer shear stress sign in tests. |
| tests/analysis/test_slab_tensile_comparisons.py | Adjusts comparison cases and adds an additional case to reflect new tensile-criterion behavior. |
| tests/analysis/test_criteria_evaluator.py | Updates the low-density threshold test to reflect directional exclusion logic. |
| tests/analysis/test_analyzer.py | Adds a regression test asserting non-negative ERR integrands for consistent stress/strain inputs. |
| src/weac/core/field_quantities.py | Updates weak-layer shear stress (tau) and related docstrings. |
| src/weac/components/criteria_config.py | Updates documentation for conditional low-density handling in the slab tensile criterion. |
| src/weac/analysis/criteria_evaluator.py | Implements directional low-density exclusion in _calculate_maximal_stresses and expands evaluate_SteadyState docstring. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| u_surface = self.fq.u(self.Z, h0=H / 2) | ||
|
|
||
| expected = -self.fq.es.weak_layer.kt * (self.Z[3, :] * h / 2 - u_surface) | ||
| expected = self.fq.es.weak_layer.kt * (self.Z[3, :] * h / 2 - u_surface) |
Comment on lines
50
to
+54
| Order of magnitude for stress envelope. Default is 1.0. | ||
| low_density_threshold_kg_m3 : float | ||
| Slab density threshold in kg/m^3 below which a layer is treated as weak snow | ||
| and counted as prone to tensile failure in the slab tensile criterion. | ||
| and excluded from the slab tensile criterion percentage when broken through | ||
| directional growth from above. |
| ## Major | ||
|
|
||
| - [ ] Layer & Slab using pipelines from Mary-Kate + Attributes (value, calculated [bool], pipeline, uncertainty) | ||
| - [ ] Uncertainties propagation |
|
|
||
| ## Minor | ||
|
|
||
| - [ ] Swap to Pytest from Unittest |
|
|
||
| ## Minor | ||
|
|
||
| - [ ] Swap to Pytest from Unittest |
Comment on lines
109
to
113
| def tau(self, Z: np.ndarray, unit: StressUnit = "MPa") -> float | np.ndarray: | ||
| """Weak-layer shear stress `tau = -kt * (w' * h/2 - u(h=H/2))`""" | ||
| """Weak-layer shear stress `tau = kt * h * (w' / 2 - u(h=H/2) / h)`""" | ||
| return ( | ||
| -self._unit_factor(unit) | ||
| self._unit_factor(unit) | ||
| * self.es.weak_layer.kt |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Documentation
Tests