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

Feature timber sections #466

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jchkoch
Copy link

@jchkoch jchkoch commented Oct 12, 2024

Feature Timber Sections

Discussion: #437

This is a start at adding timber cross-section analysis. Please let me know what you think of the approach to add timber sections. So far I have added geometry creation of layered timber sections. I have added tests for the code that I have written so far.

Moving forward I will be checking and validating the results from the cross-sectional properties calculated for the sections as well as the stress analysis modules.

  • Added tests for changed code.
  • Updated documentation for changed code.
  • Run the Nox test suite to check for errors and warnings.

@robbievanleeuwen
Copy link
Owner

Looks promising! I think this would be a worthy addition to sectionproperties.

I've also included @connorferster as a reviewer as I believe he has more timber expertise than myself!

Looking forward to reviewing the complete PR!

FYI - the docs for this PR can be found here.

@connorferster
Copy link
Collaborator

connorferster commented Oct 28, 2024

I think it looks good! @jchkoch Glad you dropped the timber rectangular section in favour of the CLT section. Setting the shear modulus of the cross-lams via the elastic_modulus and poissons_ratio is the way to do it but it sure feels weird to see it (just a general comment, not a reason to change anything).

If we can get the tests passing, then I am good to go on this one.

Edit: Just added a couple of comments.

di = float(d[idx])
layer = lay_orient[idx]

if layer is int(0):
Copy link
Collaborator

@connorferster connorferster Oct 28, 2024

Choose a reason for hiding this comment

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

This seems redundant (int(0)) and misses the mark slightly. Could it not just be if layer == 0?


Args:
d: Timber layer section thickness
lay_orient: List of layer orientation
Copy link
Collaborator

@connorferster connorferster Oct 28, 2024

Choose a reason for hiding this comment

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

I think lay_orient needs a little more explanation. The implementation of this function assumes certain values to be in there, namely 0 and any other value. The lay orientation could just as well be [0, 1, 0, 1, 0] instead of [0, 90, 0, 90, 0] and the function would work the same (it could also use [0, 34, 0, -15, 0,] and you would get the same result).

I think a list of values to describe the layup makes sense but it seems to rely, somewhat, on "magic values" that are not explained. I think it would be good to say something like:

lay_orient: A list of layer orientations, e.g. [0, 90, 0, 90, 0], where 0 indicates where `timb_mat0` is to be used and where 90 indicates where `timb_mat1` is to be used.

Maybe the user just provides the layup directly by building a list of Material, like:

    timb_mat0 = pre.Material(
        name="Timber E0",
        elastic_modulus=9.5e3,
        poissons_ratio=0.35,
        density=4.4e-7,
        yield_strength=5.5,
        color="burlywood",
    )

    timb_mat90 = pre.Material(
        name="Timber90",
        elastic_modulus=317,
        poissons_ratio=0.35,
        density=4.4e-7,
        yield_strength=5.5,
        color="orange",
    )
 layup = [timb_mat0, timb_mat90, timb_mat0, timb_mat90, timb_mat0]
 
 sect = clt_rectangular_section(
     d=[40, 40, 40],
     lay_orient=layup,
     b=1000,
)

What do you think?

@jchkoch
Copy link
Author

jchkoch commented Dec 14, 2024

@connorferster Thanks for the comments. Unfortunately, I became somewhat busy and haven't managed to follow up. However, I should have a bit more time going forward.

@robbievanleeuwen
Copy link
Owner

robbievanleeuwen commented Dec 18, 2024

Thanks for checking in @jchkoch. If you don't mind I can update your feature branch to the current state of sectionproperties, in which the CI/CD has changed a little bit since this PR was opened (not sure if you can see the "Update branch" button below- happy for you to click this if you can see it!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants