camUrban / PteraSoftware

Ptera Software is a fast, easy-to-use, and open-source software package for analyzing flapping-wing flight.
MIT License
168 stars 32 forks source link

[BUG] #14

Closed YISHISHEN closed 2 years ago

YISHISHEN commented 2 years ago

Hey there, when I run "unsteady_ring_vortex_lattice_method_solver_variable" in example, it shows:

Simulating: 1% |▍ | Elapsed: 00:01, Remaining: 01:59 Traceback (most recent call last): File "D:/soft/PteraSoftware/examples/unsteady_ring_vortex_lattice_method_solver_variable.py", line 432, in example_solver.run( File "D:\soft\PteraSoftware\pterasoftware\unsteady_ring_vortex_lattice_method.py", line 456, in run self.calculate_near_field_forces_and_moments() File "D:\soft\PteraSoftware\pterasoftware\unsteady_ring_vortex_lattice_method.py", line 962, in calculate_near_field_forces_and_moments self.calculate_solution_velocity(points=self.panel_front_vortex_centers) File "D:\soft\PteraSoftware\pterasoftware\unsteady_ring_vortex_lattice_method.py", line 816, in calculate_solution_velocity velocities_from_wings = aerodynamics.collapsed_velocities_from_ring_vortices( ZeroDivisionError: division by zero

Process finished with exit code 1

Can you have me with that? Thanks a lot.

YISHISHEN commented 2 years ago

and I'm using version2.0

camUrban commented 2 years ago

Hi @YISHISHEN. Thanks for the bug report! I'll take a look at this issue now.

camUrban commented 2 years ago

@YISHISHEN, I'm still working on replicating your bug. By the way, I just deleted your duplicate bug report on Issue #7. I think you may have added it there by accident. 😄

camUrban commented 2 years ago

Hello again! I'm having some trouble replicating your bug. I ran unsteady_ring_vortex_lattice_method_solver_variable.py without any issues. Here was my terminal output:

C:\Users\camer\Documents\GitHub\PteraSoftware\venv\Scripts\python.exe C:/Users/camer/Documents/GitHub/PteraSoftware/examples/unsteady_ring_vortex_lattice_method_solver_variable.py Simulating:100% |█████████████████████████████████████████████████| Elapsed: 00:08, Remaining: 00:00 Orient the view, then press "q" to close the window and produce the animation.

Is it possible that you changed any code in the unsteady_ring_vortex_lattice_method_solver_variable.py file? Are you able to run any of the other example files?

YISHISHEN commented 2 years ago

Thanks for your reply. I didn't change any code in all files. And I can't run any examples at all. I think I will try on another Pc to see if the environment causes the problem.

Zach10a commented 2 years ago

I can confirm I am getting the same issues on a clean install of the project in PyCharm with example steady_horseshoe_vortex_lattice_method_solver.py

Edit: Full Traceback

Traceback (most recent call last): File "C:/Users/zacht/PycharmProjects/PteraSoftware/examples/steady_horseshoe_vortex_lattice_method_solver.py", line 219, in example_solver.run( File "C:\Users\zacht\PycharmProjects\PteraSoftware\pterasoftware\steady_horseshoe_vortex_lattice_method.py", line 140, in run self.calculate_near_field_forces_and_moments() File "C:\Users\zacht\PycharmProjects\PteraSoftware\pterasoftware\steady_horseshoe_vortex_lattice_method.py", line 343, in calculate_near_field_forces_and_moments total_velocities = self.calculate_solution_velocity( File "C:\Users\zacht\PycharmProjects\PteraSoftware\pterasoftware\steady_horseshoe_vortex_lattice_method.py", line 322, in calculate_solution_velocity induced_velocities = aerodynamics.collapsed_velocities_from_horseshoe_vortices( ZeroDivisionError: division by zero

Zach10a commented 2 years ago

Looks like if you comment out the @njit from aerodynamics.py collapsed_velocities_from_horseshoe_vortices function it removes the error for some examples, however then spits out some rather bizarre values and visually looks wrong on the popup.

YISHISHEN commented 2 years ago

Looks like if you comment out the @njit from aerodynamics.py collapsed_velocities_from_horseshoe_vortices function it removes the error for some examples, however then spits out some rather bizarre values and visually looks wrong on the popup.

yeah, I have the same issues, sometimes in few examples, it can run but the visualize is still wrong.

camUrban commented 2 years ago

Hey all! Sorry about the delay. I was with my family last week. I'll dig into this more and get back to you pronto.

Zach10a commented 2 years ago

Have somehow managed to get this to spit out the correct results if you comment out the @njit line of both collapsed_velocities_from_line_vortices and collapsed_velocities_from_horseshoe_vortices, however it only works on the horseshoe example (Example 1) and is reasonably temperamental.

camUrban commented 2 years ago

Nice detective work! The @njit decorator on those functions is from the Numba package. It tells Numba to precompile those functions, which significantly speeds up the solver. One quick fix may be to comment out each Numba decorator. There are eight total (6 in aerodynamics.py and 2 in functions.py). @Zach10a and @YISHISHEN, could you try this on your end and let me know if it solves your problems?

However, I'm still struggling to reproduce this bug on my end. Numba is known to behave unexpectedly due to incorrect dependencies. Here's the list of packages installed in my virtual environment:

image image

Do you have all the same packages/versions? Also, what version of Python are you using?

In the mean time, I'll reread Numba's troubleshooting documentation to see if I can find something that explains this behavior.

camUrban commented 2 years ago

After some research, I have a hunch about what is causing the problem. Numba-compiled functions have an option to perform floating-point operations faster but at the expanse of strict IEEE 754 compliance. This option is set by the fastmath argument to the njit decorator.

Our computers may perform these slightly inaccurate operations differently, bypassing the divide-by-zero checks I placed within the Numba-compiled functions.

@Zach10a and @YISHISHEN, could you try rerunning the examples after setting the fastmath argument to False in all 8 njit decorators?

Zach10a commented 2 years ago

I have the same packages and am running Python 3.8, still no luck. Will look at that suggestion about fastmath sometime later this evening!

Zach10a commented 2 years ago

The fastmath fix works! Unsure if you want to fix this by changing them all to False or by implementing some different DIV/0 checks so will leave it to you but they all work as expected when fastmath is changed to False. For reference, I am working on Windows 11 with an Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz.

camUrban commented 2 years ago

@Zach10a, that's awesome! The fastmath attribute doesn't affect the speed that much, so I think it's worth disabling it for stability. To check that we haven't introduced any new issues, would you mind running the unit tests? If you've cloned the repo, you can do this in PyCharm by right-clicking on the tests folder and selecting "Run Python tests in test."

If all 16 tests pass on your computer, I'll push out a hotfix with this change!

Zach10a commented 2 years ago

16 for 16, you want me to PR my branch or do you just want to fix it and push it through from your own?

camUrban commented 2 years ago

I can fix it on my end and push it through! To be honest, I haven't learned how to incorporate pull requests into the versioning workflow. I'll do some research and figure that out, as it will be necessary for collaborating on future features!

Zach10a commented 2 years ago

No worries at all, I'll rescind my PR once it goes through onto the develop branch. I found https://www.petermorlion.com/incoporating-pull-requests-in-your-teams-workflow-2/ to be a helpful article, and (although I can't purport it to be a great example by any stretch) https://github.com/aliaksei135/seedpod_ground_risk/pulls?q=is%3Apr+is%3Aclosed might be helpful as a practical example. I'm no expert but the guy that taught me Git is the one that runs that repo and it (mostly) seemed to go ok.

Excited to see what the future brings for this project! Never did I think my 'Bio Flow' lectures were actually going to help...

camUrban commented 2 years ago

This issue is fixed as of v2.0.1!