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

Made Beam class accept cross-section geometries #17055

Merged
merged 11 commits into from Jun 25, 2019

Conversation

Projects
None yet
7 participants
@ishanaj
Copy link
Contributor

commented Jun 19, 2019

As discussed in PR #16964 , the beam class is made to accept geometry object as an option instead of the second moment.
Instead of the second moment, beam class now accepts a shape as an argument.
This argument shape is the description to the cross-section of the beam.
It is either the shape of the cross section (Geometry object) or the second moment of the beam cross-section.
This introduction of geometries is limited to 2D beams. We can discuss it for 3d beams further

TODO's:

  • Adding Tests
  • Beam3D

Release Notes

  • physics.continuum_mechanics
    • Beam class now accepts Geometry object representing the shape of the cross-section
@sympy-bot

This comment has been minimized.

Copy link

commented Jun 19, 2019

Hi, I am the SymPy bot (v147). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

  • physics.continuum_mechanics
    • Beam class now accepts Geometry object representing the shape of the cross-section (#17055 by @ishanaj)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.5.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

As discussed in PR #16964 , the beam class is made to accept geometry object as an option instead of the second moment.
Instead of the `second moment`, beam class now accepts a `shape` as an argument.
This argument `shape` is the description to the cross-section of the beam.
It is either the shape of the cross section (Geometry object) or the second moment of the beam cross-section.
This introduction of geometries is limited to 2D beams. We can discuss it for 3d beams further

TODO's:
- [x] Adding Tests
- [ ] ~~Beam3D~~

#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
- physics.continuum_mechanics
    - Beam class now accepts Geometry object representing the shape of the cross-section
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@ishanaj

This comment has been minimized.

Copy link
Contributor Author

commented Jun 19, 2019

@moorepants

This comment has been minimized.

Copy link
Member

commented Jun 19, 2019

I think you are on the right track. Adding a new attribute is a good idea. I'd recommend calling the new attribute cross_section. But you need to ensure backwards compatibility. I recommend not changing any existing tests, but writing the tests for the new desired functionality first. Then try ot change the class to get it to pass both set of tests. Writing the tests first will force you to think about the desired outcome more carefully.

@ishanaj

This comment has been minimized.

Copy link
Contributor Author

commented Jun 19, 2019

Sure, will work on it.

@codecov

This comment has been minimized.

Copy link

commented Jun 19, 2019

Codecov Report

Merging #17055 into master will increase coverage by 0.05%.
The diff coverage is 88.235%.

@@             Coverage Diff              @@
##            master    #17055      +/-   ##
============================================
+ Coverage   74.435%   74.485%   +0.05%     
============================================
  Files          622       622              
  Lines       161085    161099      +14     
  Branches     37811     37814       +3     
============================================
+ Hits        119904    119995      +91     
+ Misses       35861     35783      -78     
- Partials      5320      5321       +1
function of position along the beam. Alternatively ``second_moment``
can be a geometry object such as a ``Polygon`` from the geometry module
representing the shape of the cross-section of the beam. The second moment
of area will be computed from the gemetry object internally.

This comment has been minimized.

Copy link
@smichr

smichr Jun 22, 2019

Member
Suggested change
of area will be computed from the gemetry object internally.
of area will be computed from the Polygon internally.

This comment has been minimized.

Copy link
@ishanaj

ishanaj Jun 23, 2019

Author Contributor

But ellipses, and circles are also being considered, so is it okay to specify 'Polygon' here?

This comment has been minimized.

Copy link
@oscarbenjamin

oscarbenjamin Jun 23, 2019

Contributor

Polygon is too specific but there isn't a good term in the mro to describe what you want to accept:

In [22]: Polygon.mro()                                                                                                            
Out[22]: 
[sympy.geometry.polygon.Polygon,
 sympy.geometry.entity.GeometrySet,
 sympy.geometry.entity.GeometryEntity,
 sympy.sets.sets.Set,
 sympy.core.basic.Basic,
 object]

I would just use "shape" in the docstring. In any case "gemetry" is missing an "o".

ishanaj and others added some commits Jun 23, 2019

Update sympy/physics/continuum_mechanics/beam.py
Co-Authored-By: Christopher Smith <smichr@gmail.com>
Update sympy/physics/continuum_mechanics/beam.py
Co-Authored-By: Christopher Smith <smichr@gmail.com>
@ishanaj

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2019

For cases where the user tries to add a geometry object to the second_moment attribute, I think we should raise a ValueError with a message redirecting it to change the geometry object.

>>> b.second_moment = Polygon((0, 0), (a, 0), (a, b), (0, b))
ValuError: To update cross-section geometry use `cross_section` attribute
@jashan498
Copy link
Contributor

left a comment

Looks good to me, can be merged once it gets approved by others too.

@oscarbenjamin

This comment has been minimized.

Copy link
Contributor

commented Jun 24, 2019

Looks fine to me

@cross_section.setter
def cross_section(self, s):
if s:
self._second_moment = s.second_moment_of_area()[0]

This comment has been minimized.

Copy link
@moorepants

moorepants Jun 24, 2019

Member

Being that this selects the Ixx value, I think the documentation should reflect this. Say something like "If a Geometry object is used, it is assumed that the X axis of the object is aligned with the bending axis.".

This comment has been minimized.

Copy link
@ishanaj

ishanaj Jun 24, 2019

Author Contributor

I have made the change. Please have a look

This comment has been minimized.

Copy link
@moorepants

moorepants Jun 24, 2019

Member

Looks good. It can be merged with the tests pass.

@moorepants

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

Looks good to me except for the one comment I left about being explicit in the docs about whether it selects Ixx or Iyy. Very nice addition!

@ishanaj

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2019

The checks have passed. I guess we are good to go!

@moorepants moorepants merged commit a009fcb into sympy:master Jun 25, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
sympy-bot/release-notes The release notes look OK
Details
@moorepants

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

Nice work! Thanks for adjusting to all the feedback.

@moorepants moorepants added the GSoC label Jun 25, 2019

@ishanaj

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2019

Thanks!

@moorepants

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

As a companion to this, it would be nice to see more examples in the Sphinx docs showing off this new feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.