making python-modules work #3217

philbucher · 2018-10-28T20:35:37Z

With this modification the python-scripts can be subdivided into subfolders
=> now they behave as real python modules!
=> the changes necessary are very small and can be done app-by-app without breaking anything
i.e. these changes do NOT break the current mechanism, it is fully backwards compatible and can be adapted slowly

e.g. now one can do:

import KratosMultiphysics
import KratosMultiphysics.StructuralMechanicsApplication as KSM

from KratosMultiphysics import new_linear_solver_factory

from KratosMultiphysics.StructuralMechanicsApplication import structural_mechanics_analysis

# for demonstration:
from KratosMultiphysics.StructuralMechanicsApplication.solvers import custom_solver

solves #2655

I will discuss this with @jcotela in the next days, afterwards you can discuss it in the comittee

I think this also works in python2, but I have not tried it

FYI @adityaghantasala @oberbichler

loumalouomega · 2018-10-28T23:01:10Z

loumalouomega · 2018-10-28T23:01:22Z

Totally support from here

roigcarlo

Wow nice job! 👍 from my side to!

bodhinandach · 2018-10-29T10:06:13Z

This is a very nice feature!

philbucher

Some comments abt things that I am not sure are working yet

Also I think currently if there is a __init__.py in AppName/python_scripts I think it would not be executed
=> also this I think could be solved if we want this feature

philbucher · 2018-10-29T15:50:19Z

applications/StructuralMechanicsApplication/CMakeLists.txt

@@ -196,4 +196,6 @@ install(TARGETS KratosStructuralMechanicsCore DESTINATION libs )
 install(TARGETS KratosStructuralMechanicsApplication DESTINATION libs )

 # Add to the KratosMultiphisics Python module
-install(FILES "${CMAKE_CURRENT_SOURCE_DIR}/StructuralMechanicsApplication.py" DESTINATION KratosMultiphysics )
+install(DIRECTORY DESTINATION "KratosMultiphysics/StructuralMechanicsApplication")


I am not sure if this works in Win (using / for separating the path)
=> if not I think it can be easily fixed

Usually you are safe with "/". It is also a valid separator for windows.

so this is not any longer in

KratosMultiphysics/applications/StructuralMechanicsApplication?

this means that you can only make installation outside of where you compile?

I don’t understand, can you please explain in more detail what you mean?

right now on my computer i normally keep the python files in the same directory as they normally are. This is useful since when i change a file i do not need to reinstall.

I am asking if this will change after this PR or if the python files will be left where they are

clarifying on my point, it may even be good if this is changed, since it may be the first step towards propoer python packaging of Kratos...however the need of installing every time a python file is modified is a little annoying, which is the reason for which i ask

Ah now I understand
Answer: the current behavior is NOT changed, i.e. you DON'T have to reinstall after changing python files
Only the folder-structure changes a bit in the KratosMultiphysics folder
from:

KratosMultiphysics - __init__.py - StructuralMechanicsApplication.py - FluidDynamicsApplication.py

to:

KratosMultiphysics - __init__.py - StructuralMechanicsApplication __init__.py # this is the "StructuralMechanics.py"-file - FluidDynamicsApplication.py

oh, i see.

thx!

philbucher · 2018-10-29T15:51:00Z

applications/StructuralMechanicsApplication/CMakeLists.txt

-install(FILES "${CMAKE_CURRENT_SOURCE_DIR}/StructuralMechanicsApplication.py" DESTINATION KratosMultiphysics )
+install(DIRECTORY DESTINATION "KratosMultiphysics/StructuralMechanicsApplication")
+
+install(FILES "${CMAKE_CURRENT_SOURCE_DIR}/StructuralMechanicsApplication.py" DESTINATION "KratosMultiphysics/StructuralMechanicsApplication" RENAME "__init__.py")


same as above, I am not sure if this works in Win (using / for separating the path)
=> if not I think it can be easily fixed

philbucher · 2018-10-29T15:51:30Z

applications/StructuralMechanicsApplication/StructuralMechanicsApplication.py

@@ -4,7 +4,7 @@
 application_folder = "StructuralMechanicsApplication"

 # The following lines are common for all applications
-from . import application_importer
+from .. import application_importer


I am not sure if this also works in python2
=> if not I think it can be fixed with little effort

oberbichler · 2018-10-29T16:17:10Z

Nice work!

Can I also import stuff from the native (C++) module inside my python script?

philbucher · 2018-10-29T16:50:08Z

Can I also import stuff from the native (C++) module inside my python script?

I guess ...
I mean, the C-modules stuff has not changed, I think it should be possible

I will also test it again

…cripts

philbucher · 2018-11-06T10:42:03Z

@KratosMultiphysics/technical-committee I played with this some more, for me it works as intended
Summing up again the changes I propose here:

Fully backwards compatible (see first post)
minimal changes necessary
NO need to reinstall after python-files were changed!
the AppName.py-scripts are renamed to __init__.py and moved to the python-scripts-folder of the corresponding app => standard for python-modules (Note: this could also be achieved through CMake, e.g. if an app does not have the python_scripts dir so far) => therefore up to you. Moving and renaming seems to be the more natural/pythonic way
as Riccardo asked earlier, the folder-structure in the KratosMultiphysics-folder slightly changes:

from:

KratosMultiphysics
    - __init__.py
    - StructuralMechanicsApplication.py
    - FluidDynamicsApplication.py

to:

KratosMultiphysics
    - __init__.py
    - StructuralMechanicsApplication
        __init__.py # this is the "StructuralMechanicsApplication.py"-file
    - FluidDynamicsApplication.py

What is left to do: Testing in other environments => for this I pushed some "pseudo-submodules", ofc they will be removed again
Here is the file to test this. If it runs through then it means that the imports are working
python-modules-test.zip

I think we should test it on WIndows (@oberbichler @roigcarlo ) and with python2 (@maceligueta @adityaghantasala )
Thanks for your help

adityaghantasala · 2018-11-06T10:48:34Z

I am working and using this branch extensively as I write here. I will soon summarize my experience here. As of now for me it looks ok. except for a few points (supporting recursively including of subfolders ).

pooyan-dadvand · 2018-11-07T11:39:45Z

Does it work with python 2.7?

oberbichler · 2018-11-08T10:49:06Z

Works on Windows 10/Python 3.6

philbucher · 2018-11-08T17:16:36Z

@maceligueta could you please try with python 2.7?

adityaghantasala · 2018-11-08T17:43:40Z

@philbucher I get an error with python2.7 .. I think it not related to the imports but some other ...

Traceback (most recent call last):
  File "python-modules-test.py", line 12, in <module>
    import KratosMultiphysics
  File "/home/aditya/software/kratos_python2/kratos/Kratos/KratosMultiphysics/__init__.py", line 13, in <module>
    from Kratos import *
ImportError: /home/aditya/software/kratos_python2/kratos/Kratos/libs/Kratos.so: undefined symbol: _ZN6Kratos25SimpleMortarMapperProcessILi3ELi4ENS_8VariableINS_8array_1dIdLm3EEEEELNS_16HistoricalValuesE1ELS5_0EE7ExecuteEv

philbucher · 2018-11-13T08:13:28Z

We checked in personl, seems like @adityaghantasala currently cannot get his python2 installation running (unrelated to this PR)

@maceligueta it would be great if you (or someone else that has python2) can try this out 👍

maceligueta · 2018-11-14T17:44:15Z

I just compiled the branch and tested it. I tried the tests, by doing
kratos/kratos/python_scripts> python run_tests.py
but got this typical error for Python 2.7:

/home/maceli/kratos/kratos/tests/test_KratosCore.py
Traceback (most recent call last):
  File "/home/maceli/kratos/kratos/tests/test_KratosCore.py", line 12, in <module>
    import test_model_part_io
  File "/home/maceli/kratos/kratos/tests/test_model_part_io.py", line 9, in <module>
    import KratosMultiphysics.StructuralMechanicsApplication as StructuralMechanicsApplication
  File "/home/maceli/kratos/KratosMultiphysics/StructuralMechanicsApplication/__init__.py", line 10, in <module>
    application_importer.ImportApplication(application,application_name,application_folder,caller, __path__)
  File "/home/maceli/kratos/KratosMultiphysics/application_importer.py", line 21, in ImportApplication
    raise RuntimeError(msg)

File "/home/maceli/kratos/KratosMultiphysics/application_importer.py", line 21, in ImportApplication
    raise RuntimeError(msg)
RuntimeError: 
***
*    Python file /home/maceli/kratos/kratos/tests/test_model_part_io.py
*    requires KratosStructuralMechanicsApplication
*    Please import it from your main Python script, /home/maceli/kratos/kratos/tests/test_KratosCore.py
*    If your main Python script is already importing it, you may need to re-order the imports
***

The same for the Fluid Dynamics:

RuntimeError: 
***
*    Python file /home/maceli/kratos/applications/FluidDynamicsApplication/tests/artificial_compressibility_test.py
*    requires KratosExternalSolversApplication
*    Please import it from your main Python script, /home/maceli/kratos/applications//FluidDynamicsApplication/tests/test_FluidDynamicsApplication.py
*    If your main Python script is already importing it, you may need to re-order the imports
***

Solid Mechanics:

RuntimeError: 
***
*    Python file /home/maceli/kratos/applications/SolidMechanicsApplication/python_scripts/MainSolid.py
*    requires KratosExternalSolversApplication
*    Please import it from your main Python script, /home/maceli/kratos/applications//SolidMechanicsApplication/tests/test_SolidMechanicsApplication.py
*    If your main Python script is already importing it, you may need to re-order the imports
***

A different one from Structural Mechanics:

File "/home/maceli/kratos/applications//StructuralMechanicsApplication/tests/test_StructuralMechanicsApplication.py", line 43, in <module>
    from test_loading_conditions_line import TestLoadingConditionsLine as TTestLoadingConditionsLine
  File "/home/maceli/kratos/applications/StructuralMechanicsApplication/tests/test_loading_conditions_line.py", line 43
SyntaxError: Non-ASCII character '\xc2' in file /home/maceli/kratos/applications/StructuralMechanicsApplication/tests/test_loading_conditions_line.py on line 43, but no encoding declared; see http://python.org/dev/peps/pep-0263/ for details

Constitutive Models Application:

======================================================================
ERROR: test_execution (ValidationTests.Shear_Test_Von_Misses_Model)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/maceli/kratos/applications/ConstitutiveModelsApplication/tests/TestFactory.py", line 31, in test_execution
    self.test.Run()
  File "/home/maceli/kratos/applications/ConstitutiveModelsApplication/python_scripts/MainMaterial.py", line 62, in Run
    self.Initialize()
  File "/home/maceli/kratos/applications/ConstitutiveModelsApplication/python_scripts/MainMaterial.py", line 77, in Initialize
    self._construct_material_processes(self.materials)
  File "/home/maceli/kratos/applications/ConstitutiveModelsApplication/python_scripts/MainMaterial.py", line 171, in _construct_material_processes
    return (process_factory.KratosProcessFactory(self.model).ConstructListOfProcesses(material_parameters))
  File "/home/maceli/kratos/kratos/python_scripts/process_factory.py", line 14, in ConstructListOfProcesses
    kratos_module = __import__(item["kratos_module"].GetString())
ImportError: No module named ConstitutiveModelsApplication

After this, the tests hang up during PfemApplication tests. :(

Sorry if some of these errors are my bad!

philbucher · 2018-11-15T11:09:09Z

@maceligueta yes it is possible that you had to import these apps, I didn't think abt that at the time.
This does not depend on the changes of this PR though
Thanks for testing!

Iit seems to work for the configurations we are having
@KratosMultiphysics/technical-committee I guess it is worth taking a closer look now :D

philbucher · 2018-11-27T15:41:22Z

ping @KratosMultiphysics/technical-committee

jcotela

The technical committee approves. We still need to decide how to treat subfolders, but that is a separate discussion.

Attention everyone: there may be some runtime issues when switching between branches that have/don't have this change. If python complains about being unable to import some application, please delete the KratosMultiphysics folder and reinstall.

adityaghantasala · 2018-12-02T18:41:05Z

After merging this PR all the applications should be changed to adapt the CMakeLists file or ?
Will these changes be made as a part of this PR or as individual PRs ?

philbucher · 2018-12-02T18:55:38Z

@adityaghantasala please leave this PR clean, I will merge it tmr
How to proceed we can discuss in person

adityaghantasala · 2018-12-02T19:00:17Z

@philbucher sure we can discuss in person ! But ya I wanted to know about other applications as (StructuralMechanicsApplciation) is changed here ...

jcotela · 2018-12-03T08:40:46Z

@philbucher probably it would be smart to write a short how-to (here or on the wiki) so that people can port their applications on their own.

philbucher · 2018-12-03T10:18:51Z

@philbucher probably it would be smart to write a short how-to (here or on the wiki) so that people can port their applications on their own.

sure, will do

jcotela

Re-approving.

philbucher · 2018-12-03T12:51:21Z

wiki added:
https://github.com/KratosMultiphysics/Kratos/wiki/Applications-as-python-modules

philbucher · 2018-12-03T12:53:54Z

I will merge this now
If anyone has problems please let me know

I suggest to wait one or two days until using this in other places, just to see if it has some unwanted side-effects

NOTE the structuring of subfolders was not yet discussed, I suggest to wait a bit more and let the @KratosMultiphysics/technical-committee decide how the subfolders should look like

ddiezrod · 2018-12-06T16:36:51Z

FYI @KratosMultiphysics/altair

ddiezrod · 2018-12-06T16:37:08Z

FYI @KratosMultiphysics/altair

philbucher added 2 commits October 28, 2018 21:27

making python-modules work

3261b4f

adding a comment

385eb8c

philbucher added Enhancement Discussion Kratos Core Applications Python labels Oct 28, 2018

philbucher requested a review from a team October 28, 2018 20:35

roigcarlo reviewed Oct 29, 2018

View reviewed changes

philbucher commented Oct 29, 2018

View reviewed changes

philbucher added 5 commits November 6, 2018 09:38

Merge branch 'master' into feature-python-modules

5579706

renaming StructuralMechanics.py to __init__.py and moving to python-s…

541af8f

…cripts

simplifying cmake-lists

7670994

adding submodules FOR TESTING ONLY, TO BE REVERTED

0e14da0

minor

162de93

Merge branch 'master' into feature-python-modules

0d8731c

adityaghantasala mentioned this pull request Nov 16, 2018

[WIP][TO-DISCUSS] Co sim app #3390

Closed

jcotela previously approved these changes Nov 30, 2018

View reviewed changes

philbucher added 2 commits December 2, 2018 16:47

Merge branch 'master' into feature-python-modules

74f7cd1

deleted testing-files again

cfffd70

philbucher dismissed jcotela’s stale review via cfffd70 December 2, 2018 15:50

jcotela approved these changes Dec 3, 2018

View reviewed changes

philbucher merged commit fcc349c into master Dec 3, 2018

philbucher deleted the feature-python-modules branch December 3, 2018 12:56

This was referenced Dec 3, 2018

application_importer : provide folders to be included #2655

Closed

[Core][Testing] change in detection of apps in test-runner #3567

Merged

jcotela mentioned this pull request Dec 5, 2018

Fluid/module init #3584

Merged

This was referenced Dec 11, 2018

more apps as python-modules #3632

Merged

Apps/improve py init #3642

Merged

bodhinandach mentioned this pull request Dec 18, 2018

[ParticleMechanics] Python modules init #3689

Merged

philbucher mentioned this pull request Jan 16, 2019

Cleaning up Python module import infrastructure #3608

Merged

miguelmaso mentioned this pull request Jan 3, 2022

[ShapeOpt] add missing init files #9507

Merged

making python-modules work #3217

making python-modules work #3217

Conversation

philbucher commented Oct 28, 2018

loumalouomega commented Oct 28, 2018

loumalouomega commented Oct 28, 2018

roigcarlo left a comment

Choose a reason for hiding this comment

bodhinandach commented Oct 29, 2018

philbucher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oberbichler commented Oct 29, 2018

philbucher commented Oct 29, 2018

philbucher commented Nov 6, 2018 • edited Loading

adityaghantasala commented Nov 6, 2018

pooyan-dadvand commented Nov 7, 2018

oberbichler commented Nov 8, 2018

philbucher commented Nov 8, 2018

adityaghantasala commented Nov 8, 2018

philbucher commented Nov 13, 2018 • edited Loading

maceligueta commented Nov 14, 2018 • edited Loading

philbucher commented Nov 15, 2018

philbucher commented Nov 27, 2018

jcotela left a comment

Choose a reason for hiding this comment

adityaghantasala commented Dec 2, 2018

philbucher commented Dec 2, 2018

adityaghantasala commented Dec 2, 2018

jcotela commented Dec 3, 2018

philbucher commented Dec 3, 2018

jcotela left a comment

Choose a reason for hiding this comment

philbucher commented Dec 3, 2018

philbucher commented Dec 3, 2018

ddiezrod commented Dec 6, 2018

ddiezrod commented Dec 6, 2018

philbucher commented Nov 6, 2018 •

edited

Loading

philbucher commented Nov 13, 2018 •

edited

Loading

maceligueta commented Nov 14, 2018 •

edited

Loading