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

[GSOC] Draw method for Cable class #26640

Merged
merged 27 commits into from
Jun 20, 2024
Merged

Conversation

shishir-11
Copy link
Contributor

@shishir-11 shishir-11 commented May 30, 2024

References to other Issues or PRs

Brief description of what is fixed or changed

This PR aims to track the progress of Phase 1 of GSOC project aimed to Extend the continuum_mechanics module.
Draw method is implemented for Cable and changes to the solver for consistency of equation with the lowest point.
Plot for tension also added .

Other comments

Release Notes

  • physics.continuum_mechanics
    • A draw method to plot the Cable diagram as specified by the user
    • Changes to solver for consistency with lowest point
    • Plot for tension added

@sympy-bot
Copy link

sympy-bot commented May 30, 2024

Hi, I am the SymPy bot. 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:

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

Click here to see the pull request description that was parsed.
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed
This PR aims to track the progress of Phase 1 of GSOC project aimed to Extend the continuum_mechanics module.
Draw method is implemented for Cable and changes to the solver for consistency of equation with the lowest point.
Plot for tension also added .

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers. Formerly, `log(-x)` incorrectly gave `-log(x)`.

* physics.units
  * Corrected a semantical error in the conversion between volt and statvolt which
    reported the volt as being larger than the statvolt.

or if no release note(s) should be included use:

NO ENTRY

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
  * A draw method to plot the Cable diagram as specified by the user 
  * Changes to solver for consistency with lowest point
  * Plot for tension added
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@shishir-11
Copy link
Contributor Author

@Ishanned @AdvaitPote this draft PR will track my Draw method progress. I will make a final PR when eveything is ready.
Please take a look at the discussion on Gitter as whenever I include rectangles, I am getting an error. I also get an error in my interactive shell when I use other draw methods. Any idea why that might be ?

Copy link

github-actions bot commented May 31, 2024

Benchmark results from GitHub Actions

Lower numbers are good, higher numbers are bad. A ratio less than 1
means a speed up and greater than 1 means a slowdown. Green lines
beginning with + are slowdowns (the PR is slower then master or
master is slower than the previous release). Red lines beginning
with - are speedups.

Significantly changed benchmark results (PR vs master)

Significantly changed benchmark results (master vs previous release)

Full benchmark results can be found as artifacts in GitHub Actions
(click on checks at the top of the PR).

@shishir-11
Copy link
Contributor Author

shishir-11 commented Jun 10, 2024

Can you please check its working out once, I will move forward to adding the docstring for the new method and its usage if the diagrams are satisfactory.

@AdvaitPote
Copy link
Member

AdvaitPote commented Jun 10, 2024

The Supports seem to be changing as per the scale of the diagram/plot. Try and avoid this. Also, the distributed loads seem to look uneven too in different plots. In my opinion, the two plots should be a few millimeters apart at best. The second case is missing the force magnitude labelling too.

Figure_1

Figure_2

Figure_3

@shishir-11
Copy link
Contributor Author

shishir-11 commented Jun 11, 2024

Shape of cable under UDL
It takes a parabolic shape under uniformly distributed loads if weight of cable is not considered. However in the current solver it allows taking a random lowest point, in the solver coefficients of a parabolic equations are calculated

however the method does not ensure the provided lowest point will be the lowest point of the formed equation. For example, in the case of same height of supports the solver would accept lowest_x as values other than the midpoint of left and right support and still produce a result which probably shouldn't happen.

In [1]: from sympy.physics.continuum_mechanics.cable import Cable
   ...: c=Cable(("A", 0, 10),("B", 20,10))
   ...: c.apply_load(0, ("X", 850))
   ...: c.solve(5,0)

In [2]: c.tension
Out[2]: {'distributed': 19125*sqrt((4*X/15 - 4/3)**2 + 1)/2}

In [3]: c.draw().show()

I have fixed the issue with the magnitude symbols disappearing and bad support shapes. For running draw use the order given above as without using solve , draw does not work.

@shishir-11 shishir-11 marked this pull request as ready for review June 11, 2024 23:44
@Ishanned
Copy link
Contributor

Hi @shishir-11, can you share what happens if you call the draw method without calling solve first?

@shishir-11
Copy link
Contributor Author

shishir-11 commented Jun 14, 2024

Hi @shishir-11, can you share what happens if you call the draw method without calling solve first?

The solve method takes arguements for the lowest point of the cable which is used to form the equation later. So calling draw without solve works fine for point loads , for distributed loads we would get an error trying to access the lowest point coordinates.

@AdvaitPote
Copy link
Member

>>> from sympy.physics.continuum_mechanics.cable import Cable
>>> c = Cable(("A", 0, 10), ("B", 10, 10))
>>> c.apply_load(-1, ('Z', 2, 7.26, 3, 270))
>>> c.apply_load(-1, ('X', 4, 6, 8, 270))
>>> c.solve()
>>> c.draw().show()

I just ran this example locally. I am getting a good and correct diagram as expected. However, it's giving me a Runtime Warning: RuntimeWarning: invalid value encountered in wrapper_func (vectorized) outputs = ufunc(*inputs)
It's just a warning but still, it'll be better to maybe inspect a little. Kindly check if the function is encountering any None or infinite values in its computations.

@shishir-11
Copy link
Contributor Author

Can

>>> from sympy.physics.continuum_mechanics.cable import Cable
>>> c = Cable(("A", 0, 10), ("B", 10, 10))
>>> c.apply_load(-1, ('Z', 2, 7.26, 3, 270))
>>> c.apply_load(-1, ('X', 4, 6, 8, 270))
>>> c.solve()
>>> c.draw().show()

I just ran this example locally. I am getting a good and correct diagram as expected. However, it's giving me a Runtime Warning: RuntimeWarning: invalid value encountered in wrapper_func (vectorized) outputs = ufunc(*inputs) It's just a warning but still, it'll be better to maybe inspect a little. Kindly check if the function is encountering any None or infinite values in its computations.

I'm unable to reproduce this warning on my system , can you provide any specifications or method to reproduce this.

@AdvaitPote
Copy link
Member

AdvaitPote commented Jun 16, 2024

I'm unable to reproduce this warning on my system , can you provide any specifications or method to reproduce this.

That's odd. If it's something with my device then no problem. Just focus on the 'adding distributed loads' feature. I am looking for possible changes and might post another review.

@shishir-11
Copy link
Contributor Author

I have implemented the 'adding distributed load' feature in an earlier PR, please check.

mag = 0
for key in self._loads['distributed']:
    mag += self._loads['distributed'][key]

force_arrows.append(
    {
        'text':f'{mag} N/m',
        'xy':((self._left_support[0]+self._right_support[0])/2,self._lowest_y_global - max_diff*0.15)
    }
)

@AdvaitPote
Copy link
Member

I don't see any more changes function wise. @Ishanned ?

@Ishanned
Copy link
Contributor

I don't see any more changes function wise. @Ishanned ?

I'll have a final look and let you know

@Ishanned
Copy link
Contributor

LGTM. After yesterday's discussion, I think we should merge this PR.

@Ishanned Ishanned merged commit 7293029 into sympy:master Jun 20, 2024
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants