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

basic linux support #560

Merged
merged 3 commits into from
Nov 18, 2022
Merged

Conversation

harshasunder-1
Copy link
Contributor

Hello!

this is with regard to issue #104
The latest pyfemm supports linux, so I merged that into pyleecan.
From my tests, it supports drawing in femm properly, as well as simulation, though it is slow.

Some of the things that I noticed didnt work are :-

  1. the get_meshsolution function didnt work, but I think the errors were only to do with the filepaths.
    Can we have a global isWindows flag somewhere? Maybe in the _FemmHandler itself?

  2. parallelization didnt work - I think thats because in Linux we need to create a file in the same directory as femm to transfer the command, so if you open multiple FEMMs its not clear which instance is opening or deleting the file. A quick though ugly hack might be to just have multiple copies of the FEMM folder.

Thank you
Harsha

@RaphaelPile
Copy link
Contributor

Hello there !
Thank you very much for this contribution.
It is not really code related, but could you provide some details on how you managed to "install" FEMM on your Linux OS ?

I am working with Mac OS, and I want to check if your PR is working on my side as well.

Best,

Raphaël

@harshasunder-1
Copy link
Contributor Author

Hello!
I just installed it through wine. If you look at openFemm function inside _FemmHandler.py file (in the Classes subdirectory), there are some default paths for linux that the code searches for FEMM (including the wine dir). The new function even takes a path to the exe as an argument. Maybe for MacOS you might need to change those paths, or feed the path into the function as an argument
Thanks,
Harsha

@RaphaelPile
Copy link
Contributor

Okay thanks. For unknown reason, I was not able to use Wine. I am currently able to run femm GUI, but I still have to look how to make it work with pyfemm. I will let you know when I can test your PR.

@BonneelP
Copy link
Collaborator

Hello,

I checked your modifications and it looks fine to me. All the tests are passing on Windows including with parallelization.
I would only have two readability improvements to suggest:

  • What about renaming windowOS to is_window_OS ?
  • Do you think that you could split the modified methods into X_Win and X_Linux with an encapsulating method calling the correct version according to windowOS ? In particular for openfemm, callfemm and callfemm_noeval
    I should have time to do both myself if needed.

@RaphaelPile When do you think you would have time to check if it works on your side ? I'm currently not using Linux so I would be a great help if you could validate these modifications :) Otherwise I will take time to install Linux but I don't know when.

By curiosity what are the methods "X_setcomment" ? Are they methods introduced by pyfemm 0.1.3 ? I think we haven't updated the _FEMMHandler for a long time and I somehow managed to forget where this file came from in the first place :/

Thanks again for your contribution,
Pierre

@harshasunder-1
Copy link
Contributor Author

Hello,
Sorry for the delay. I have made the changes you suggested, it makes the code much more readable.
the new X_setcomment functions allow you to "set the comment text in the problem definition". They were adding in the latest pyfemm update
Unfortunately its still very slow in linux. You can use it to draw the machine, but very tough to use it for doing simulations and impossible for optimizations I think.
Thank you,
Harsha

@RaphaelPile
Copy link
Contributor

@BonneelP I don't know, I have already issues to configure VSCode for python on my Mac OS. I will try my best to do it before the end of October.

@BonneelP
Copy link
Collaborator

Hello,

I didn't manage to try your modifications on Linux, but all the tests are still Ok on windows and I'm about to create a new release to solve issue #569 so I'm (finally) merging your modification. Sorry for the long time to merge your contribution, it will be really helpful for the community.

I will still try to find some time to use your code on Linux to see if we can solve the speed issue.

Best regards,
Pierre

@BonneelP BonneelP merged commit 1aff9d5 into Eomys:master Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants