Skip to content
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

#355 Added menard equations 2.21 and 2.22 of CUR 228 #356

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

johan-tuls
Copy link
Contributor

Description

Added Menard equations CUR 228

Please delete options that are not relevant.

  • [X ] New feature (non-breaking change which adds functionality)
  • [ X] This change requires a documentation update

Checklist:

  • [ X] I have added tests that prove my fix is effective or that my feature works
  • [ X] I have commented my code, particularly in hard-to-understand areas
  • [ X] I have made corresponding changes to the documentation
  • [ X] New and existing unit tests pass locally with my changes

@johan-tuls johan-tuls linked an issue Oct 16, 2024 that may be closed by this pull request
2 tasks
Copy link

Thank you so much for contributing to Blueprints! This is your Pull Request # 2 to this project.
Your contributions help thousands of engineers work more efficiently and accurately.

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!

@johan-tuls johan-tuls changed the title #355 Added menard equations 2.21 and 2.22 of CUR 288 #355 Added menard equations 2.21 and 2.22 of CUR 228 Oct 16, 2024
Copy link

@rickdegoeij rickdegoeij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bit nitpicky sometimes, so feel free to interpret the comments :)

Comments that need address:

  • Latex equations don't match implementation
  • unittests for negative values
  • add negative value checks in 2.22
  • unittest for correctness of latex equations

@staticmethod
def _evaluate(r: M, e_p: KPA, alpha: float) -> KN_M3:
"""Return the Menard stiffness k_h when r >= 0.3 m [kN/m3]."""
if r < 0.3:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here 0.3 is hardcoded, but in 2.21 it's a static value. Maybe refer to the same constant in a constant file

"""Return the Menard stiffness k_h when r >= 0.3 m [kN/m3]."""
if r < 0.3:
return e_p / 2 / r / ((4 * 2.65**alpha + 3 * alpha) / 18)
msg = "Radius is equal to- or larger than 0.3m, use: Eq2Dot21MenardStiffness"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if using constant maybe change this to a f-string and then put the r_0 in there

return LatexFormula(
return_symbol=r"k_{h}",
result=f"{self:.{n}f} kN/m3",
equation=r"\frac{2 \cdot R}{E_{p}} \cdot \frac{4 \cdot 2.65^{\alpha} + 3 \alpha}{18}",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the original equation, but the return symbol is k_h

the original 2.22 returns 1 / k_h


return LatexFormula(
return_symbol="k_{h}",
equation=r"\frac{1}{3 \cdot E_{p}} \cdot "

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the original equation, but the return symbol is k_h

the original 2.21 returns 1 / k_h

rf"\left[1.3 \cdot {self.r_0 :.{n}} "
rf"\left( 2.65 \cdot \frac{{{self.r :.{n}}}}{{{self.r_0 :.{n}}}}\right)^{{{self.alpha :.{n}f}}}"
rf"+ {self.alpha :.{n}} \cdot {self.r :.{n}}\right]",
result=f"{self.result :.{n_decimals}f}",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n_decimals used instead of n. (doesn't change result)

from blueprints.codes.cur.cur_228.formula_2_21 import Form2Dot21MenardStiffness


class TestForm2Dot21MenardStiffness:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add test with invalid values for e_p


@staticmethod
def _evaluate(r: M, e_p: KPA, alpha: float) -> KN_M3:
"""Return the Menard stiffness k_h when r >= 0.3 m [kN/m3]."""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add checks for negative numbers

Suggested change
"""Return the Menard stiffness k_h when r >= 0.3 m [kN/m3]."""
"""Return the Menard stiffness k_h when r >= 0.3 m [kN/m3]."""
raise_if_negative(r=r, e_p=e_p, alpha=alpha)

from blueprints.codes.cur.cur_228.formula_2_22 import Form2Dot22MenardStiffness


class TestForm2Dot22MenardStiffness:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably add some tests for the raise if negative checks. To make sure it's captured

Comment on lines 6 to 15
from blueprints.type_alias import KN_M3, KPA, M


class Form2Dot22MenardStiffness(Formula):
"""Representation of equation 2.22 CUR 228."""

source_document = CUR_228
label = "2.22"

def __init__(self, r: M, e_p: KPA, alpha: float) -> None:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use dimensionless instead of float

Suggested change
from blueprints.type_alias import KN_M3, KPA, M
class Form2Dot22MenardStiffness(Formula):
"""Representation of equation 2.22 CUR 228."""
source_document = CUR_228
label = "2.22"
def __init__(self, r: M, e_p: KPA, alpha: float) -> None:
from blueprints.type_alias import KN_M3, KPA, M, DIMENSIONLESS
class Form2Dot22MenardStiffness(Formula):
"""Representation of equation 2.22 CUR 228."""
source_document = CUR_228
label = "2.22"
def __init__(self, r: M, e_p: KPA, alpha: DIMENSIONLESS) -> None:

Comment on lines 6 to 17
from blueprints.type_alias import KN_M3, KPA, M
from blueprints.validations import raise_if_negative


class Form2Dot21MenardStiffness(Formula):
"""Representation of equation 2.21 CUR 228."""

source_document = CUR_228
label = "2.21"
r_0: M = 0.3

def __init__(self, r: M, e_p: KPA, alpha: float) -> None:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use DIMENSIONLESS instead of float

Suggested change
from blueprints.type_alias import KN_M3, KPA, M
from blueprints.validations import raise_if_negative
class Form2Dot21MenardStiffness(Formula):
"""Representation of equation 2.21 CUR 228."""
source_document = CUR_228
label = "2.21"
r_0: M = 0.3
def __init__(self, r: M, e_p: KPA, alpha: float) -> None:
from blueprints.type_alias import KN_M3, KPA, M, DIMENSIONLESS
from blueprints.validations import raise_if_negative
class Form2Dot21MenardStiffness(Formula):
"""Representation of equation 2.21 CUR 228."""
source_document = CUR_228
label = "2.21"
r_0: M = 0.3
def __init__(self, r: M, e_p: KPA, alpha: DIMENSIONLESS) -> None:

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.

[✨ Feature request]: Menard stiffness cur 228
4 participants