Eomys / pyleecan

Electrical engineering open-source software providing a user-friendly, unified, flexible simulation framework for the multiphysic design and optimization of electrical machines and drives
https://www.pyleecan.org
Apache License 2.0
152 stars 127 forks source link

Jupyter notebooks, plotting and pyside2 #525

Open mjfwest opened 2 years ago

mjfwest commented 2 years ago

The compatibility with notebooks seems to be very broken now, which is a pity. I have recently upgraded from a fork based around 5daa0729338643e9a412c697c258b7be781162f8 where it did work, better at least.

To get the notebooks in the tutorial directory to work one first needs to change %matplotlib notebook to %matplotlib inline and change all the calls to .plot() to .plot(is_show_fig=False)

Once that is done tuto_Machine.ipynb works. However tuto_Operating_point.ipynb fails with the error:

ImportError: Can't use Winding method comp_connection_mat: 
    Importing PyQt5 disabled by IPython, which has
    already imported an Incompatible QT Binding: pyside2
BonneelP commented 2 years ago

Hello,

Thank you for having investigated this issue.

Sadly I haven't had much time for pyleecan recently (although several important PR were merged) but hopefully I will be able to come back to the project on next week and I will try to move forward to a new release 1.4.0 with all the latest modifications, a working SDT GUI and corrected tutorial/installation.

This issue is linked to swat-em (the package to compute the winding with star of slot) that requires PyQt5 (although we don't use its GUI). As pyleecan uses PySide2 it creates a conflict and the package can't be imported (in the notebook, it works in scripting) then no winding can be defined. In some part of the code we managed to correct it "the dirty way" by having a "from PySide2.QWidgets import *" before any import to swat-em to make sure that the widgets are the PySide2 ones and not the PyQt5 ones. We haven't had the time to investigate how to correct this import in the notebook nor how we could better handle this conflict but all help on the topic is welcome.

Best regards, Pierre

mjfwest commented 2 years ago

My fairly brutal solution so far, is to fork my own swat-em and comment out the plotting as I (now assume) it is not used? It seems to be an improvement and this has moved me past that point. However I'm interested to hear what it would take to completely divorce the pyleecan code from any gui? I generally see them as two separate projects? In case it is relevant, the problem is now

ImportError: Can't use MagFEMM method comp_flux_airgap: DLL load failed while importing win32api: The specified module could not be found.

This has worked previously, so I may fix it without another Issue report.

mjfwest commented 2 years ago

In response to my DLL load failed while importing win32api: , if using Anaconda, or miniconda the simplest solution is to pip uninstall win32 and reinstall with conda. pip uninstall pywin32 conda install pywin32

It feels like this isn't getting to the source of the problem, but it works.

https://stackoverflow.com/questions/58612306/how-to-fix-importerror-dll-load-failed-while-importing-win32api

mjfwest commented 2 years ago

Developing the original issue further, the first problem as @BonneelP suggests is due to swat-em importing PyQt5 when it is not needed. My solution above removes that. The 2nd issue is probably not directly related but is solved as described above. The original issue still remains and now manifests as illustrated fully in this gist but summarised here

Ploting Issues

The first issue is to make Pyleecan plot in the notebook. This seems to require you to use matplotlib yourself before importing pyleecan . If pyleecan is imported before making your first plot, it will try an open a separate QT plotting window and then crash Python hard. The partial solution is strangely to make a plot first, before importing pyleecan.

The second related issue is Pyleecan seems to overrule the backend.

So: 1) If I run First_code_cell cell first, then the Second_code_cell the notebook runs, but you can see from the plot that the backend selected by %matplotlib inline or set explicitly by a call to matplotlib.use('module://matplotlib_inline.backend_inline') gets ignored and the QT5Agg backend is used.

2) If I run Second_code_cell first the notebook hangs with an open but non responsive window. The console message is [IPKernelApp] WARNING | No such comm:

it should be noted that I have not made any calls to the GUI

BonneelP commented 2 years ago

I just checked the following configuration: image and I got: image Updating SciDataTool (and SciDataTool only) creates the bug. So I guess that the issue is coming from SciDataTool GUI in fact

image image

BonneelP commented 2 years ago

I forgot to say that between these two versions of SciDataTool we have added a GUI and so a dependency to PySide2 which could explain the conflict that we didn't had before.

BonneelP commented 2 years ago

Regarding the change of backend, it is set explicitly twice in the code (maybe it's not a good usage): https://github.com/Eomys/SciDataTool/blob/master/SciDataTool/GUI/__init__.py

https://github.com/Eomys/pyleecan/blob/master/pyleecan/GUI/__init__.py

So I guess that both import pyleecan and import SDT from Second_code_cell change the backend

BonneelP commented 2 years ago

To get the notebooks in the tutorial directory to work one first needs to change %matplotlib notebook to %matplotlib inline and change all the calls to .plot() to .plot(is_show_fig=False)

Once that is done tuto_Machine.ipynb works. However tuto_Operating_point.ipynb fails with the error:

In fact, I think that tuto_Machine also fails with this correction. There are two bugs to solve: The first one is: ImportError: Can't use Winding method comp_connection_mat: Importing PyQt5 disabled by IPython, which has already imported an Incompatible QT Binding: pyside2 It happens when trying to import swat-em in comp_connection_mat.

The second one is having the plot integrated in the notebook. I think that the above solution corrects the second but not the first one:

image As you can see the Prius winding is fully red because when we are not able to compute the winding pyleecan sets all active surface to Phase A to enable the plot.

BonneelP commented 2 years ago

Setting twice %matplotlib notebook seems to do the trick: image

It's a quick fix, we still need to find the explanation and a proper correction.

However I'm interested to hear what it would take to completely divorce the pyleecan code from any gui? I generally see them as two separate projects?

For both pyleecan and SciDataTool the GUI are not mandatory:

When I see the trouble we had to integrate swat-em, I'm starting to think about finding a way to have both GUI as an option/separate project. Maybe we are causing the same trouble as we had with swat-em to people that want to import pyleecan for some simulation (but don't want to use the GUI) and pyleecan creates conflict with an unused PySide2 import. Do you have any suggestion on how we could organize that ?

Best regards, Pierre

BonneelP commented 2 years ago

Hello, We have now removed the GUI from SciDataTool but there was still an issue. We have further investigated and it was linked to swat-em dependency to PyQt5. We have opened a PR to replace this dependency by qtpy: https://gitlab.com/martinbaun/swat-em/-/merge_requests/12 Now pyleecan requirements use this PR as a dependency for swat-em by waiting the merge and it seems that the notebook are back to normal :) I have updated the notebook and the website. Do we need to further investigate or can we close this issue ?

Best regards, Pierre