simphony / simphony-openfoam

The implementation of the SimPhoNy OpenFOAM -wrappers.
GNU General Public License v2.0
2 stars 0 forks source link

Migration of24 #61

Closed khiltunen closed 8 years ago

khiltunen commented 8 years ago

This PR extends wrappers to function on OpenFoam 2.3 and 2.4 versions. Also new mixture model solver is added and some reported issues are fixed.

khiltunen commented 8 years ago

@nathanfranklin do you or some one else has time to review this PR ?

nathanfranklin commented 8 years ago

@khiltunen , sorry. I don't have time at the moment so someone else will need to review this PR.

Note that it looks like a lots of object files as well as IDE-related files (qtcreator) were inadvertently committed.

khiltunen commented 8 years ago

Okey. Thank's. I'll remove those extra files. @TobiasRasp do you have to review ?

TobiasRasp commented 8 years ago

I will check it tomorrow. Looking forward to test my example with the new version!

TobiasRasp commented 8 years ago

Ok, in general it seems to me that the openfoam wrapper is really a powerful and convenient tool now. I checked the wrapper with OpenFoam 2.3.7 and had no longer problems with any version mismatch.

However, I've some points I'd like to mention:

  1. The installation/setup procedure includes an internal call of an extra setup (in the install_foam_interface.sh script). On my system (and I guess some others will also face this problem), I don't have root permission, which means that I always have to append "--user". This is no problem at all when starting the major setup in the shell, but the internal setup failed with no interruption of the procedure and it was hard to find the cause of issues when trying to work with the wrapper later. Maybe there should at least be a hint in the documentation that install_foam_interface.sh must be modified accordingly for the case of missing root permissions.
  2. The unit tests fail on my system when trying "python -m unittest discover" after the installation. Reason for this was the relative reference with the dot-notation in some import commands. can you please double check if the unittests work? Starting the individual tests manually worked fine by the way.
  3. For the internal wrapper the log-output is displayed on the screen. This is a little bit annoying after a while. Is it possible to redirect the log-output to a file or is this something not wanted?

4a. In foam_files.py, the value of writeInterval is nOfTimeSteps but should be nOfTimeSteps*deltaT. (internal and control wrapper) 4b. In the control wrapper, the value of startFrom is latestTime (foam_templates.py). However, it is startTime in the case of the internal wrapper (foam_dicts.py). Is this intentionally and something like a restart is not provided to be possible for the internal wrapper since no output folders exist?

  1. There is a mismatch concerning convergence and the solution between the internal and the control wrapper! For my simple Poiseuille flow example (please find the links to the files below. Actually these are py-files but for uploading reasons it was necessary to make them txt.), when using a timestep of deltaT = 10^-5, the control wrapper works fine but the Courant number explodes in the internal wrapper. Going to smaller time steps, e.g. deltaT = 10^-8, both wrapper converge, but the results are completely different. Unfortunately, I was not able to find out what's the reason for that. Ma

ybe you can help me? My suspicion is the varying fvSolution and fvSchemes or some dimension mismatch for the pressure files. For checking the result of each wrapper case, the velocity profile is generated at the end of the simulation.

poiseuille_wrapperTest_OF.txt test_mesh_dict.txt

santiagomarquezd commented 8 years ago

Hi @TobiasRasp, thanks for your evaluation, we've worked and learned a lot, Juan in coding and design, Norberto in managing and giving the gidelines of the project and me and in design and FOAM classes mastering. We are in vacations now, and we'll work on the changes as soon as posible. Regarding your comments:

  1. Root permissions shouldn't be required since all the installation is done within an independent environment
  2. OK
  3. We agreed with @khiltunen to send this output to a log file. I can't remember if this option was finally missed or temporally deactivated for debugging purposes.

4a. It dependends on the type of time control selected for data dumping, time o time steps, in internal wrapper it's irrelevant.

4b. It's equivalent in internal wrrapper since each calculation is like running from zero time.

  1. Ok, we'll test the case in both wrappers. The solutions should be equal to machine precision.

Regards!

khiltunen commented 8 years ago

Thank's for the review @TobiasRasp.

  1. I'm not sure that the current installation of the interface using setuptools is the right way to go, but you are right that now it tries to install to system directories while the --user parameter is not passed to setup on installation script. I'll try to change that.
  2. Could you give the error message from unittests. Unittests are passed on travis-ci and I'm not getting any errors.
  3. @jmarcelogimenez is there a flag in interface to deactivate the output ? 4a. This is a bug. I will change that in IO wrapper. The writeControl is set to adjustableRunTime and so the writeInterval must be nOfTimeSteps*deltaT 4b. IO wrapper uses "standard" fvSolution and fvSchemes files for pimpleFoam used in tutorials. I can see that there is some difference in internal wrapper foam_dicts.py. Could you check this @jmarcelogimenez . I'll go through your example and try to figure out where the difference is coming.
TobiasRasp commented 8 years ago

About the point 2: I realized just now that the problem only arises if "python -m unittest discover foam_controlwrapper" is performed. Then an error message as the following is given:

ERROR: tests.test_foam_files (unittest.loader.ModuleImportFailure) ImportError: Failed to import test module: tests.test_foam_files Traceback (most recent call last): File "/usr/lib64/python2.7/unittest/loader.py", line 254, in _find_tests module = self._get_module_from_name(name) File "/usr/lib64/python2.7/unittest/loader.py", line 232, in _get_module_from_name import(name) File "/W5/ras/SimPhoNy/Kopplung_CFD-MD/OF_wrapper_unittest/simphony-openfoam/foam_controlwrapper/tests/test_foam_files.py", line 17, in from foam_controlwrapper import foam_files File "/W5/ras/SimPhoNy/Kopplung_CFD-MD/OF_wrapper_unittest/simphony-openfoam/foam_controlwrapper/foam_controlwrapper.py", line 13, in from .cuba_extension import CUBAExt ValueError: Attempted relative import in non-package

If the normal "python -m unittest discover" is used instead, everything is alright! So I think you can forget about this point!

khiltunen commented 8 years ago

@TobiasRasp it seems that the transportproperties are still not updated in internal wrapper and this causes the different behaviour with io wrapper. I looked at the code and singlePhaseModel is instantiated with a value 0.01 for nu, but not updated with the values in transportProperties dictionary. @jmarcelogimenez could you check this out and make corresponding changes.

khiltunen commented 8 years ago

@TobiasRasp I made modifications to setup to work with --user option. I also corrected the bug concerning writeInterval in IO wrapper.

TobiasRasp commented 8 years ago

@khiltunen and @santiagomarquezd Thanks for your quick answers and fixes!

khiltunen commented 8 years ago

@jmarcelogimenez and @TobiasRasp i made changes to pimpleFoam internal interface to get the update of transportproperties work. It works now only with constant property transportModels, but maybe @jmarcelogimenez can find a sophisticated way to cover potens law transportModels also.

norbertonigro commented 8 years ago

Kai, we can think about how to extend the transportProperties to cover at least more transortModels but as Santiago told you we are on vacations right now so probably we will answer this issue on February.

norbertonigro commented 8 years ago

Kai, we can think about how to extend the transportProperties to cover at least more transortModels but as Santiago told you we are on vacations right now so probably we will answer this issue on February.

khiltunen commented 8 years ago

This can wait to February ( the extension to other transportModels). To remember this @jmarcelogimenez or @norbertonigro could you make a new issue to github on this ? Sorry to bother you on vacation.

khiltunen commented 8 years ago

@TobiasRasp i've modifications to interface to cover tensor treatment. Also changes to mixture model and change to boundary treatment which makes execution faster. Note that this will be changed in the coming version's when boundary faces are treated as direct groups not filtered from face data. Could you review the changes (and also confirm the earlier fixes to setup and updation of transporproperties in internal wrapper) so that we can merge this PR ?

TobiasRasp commented 8 years ago

I can already confirm the fixes concerning setup and transportProperties update (this works fine now) and will have a look on the other changes now.

TobiasRasp commented 8 years ago

@khiltunen There is something wrong with the call of foamface.modifyNumerics(mesh.name, fvSchemesDict, fvSolutionDict, controlDict) in foam_dicts.py. Running the unittests results in the following error message:

ERROR: test_run_time (foam_internalwrapper.tests.test_internalwrapper.FoamInternalWrapperRunTestCase)

Test that field variable value is changed after

Traceback (most recent call last): File "/W5/ras/SimPhoNy/Kopplung_CFD-MD/OF_wrapper_unittest/simphony-openfoam/foam_internalwrapper/tests/test_internalwrapper.py", line 183, in test_run_time self.wrapper.run() File "/W5/ras/SimPhoNy/Kopplung_CFD-MD/OF_wrapper_unittest/simphony-openfoam/foam_internalwrapper/foam_internalwrapper.py", line 63, in run modifyNumerics(mesh, self.SP) File "/W5/ras/SimPhoNy/Kopplung_CFD-MD/OF_wrapper_unittest/simphony-openfoam/foam_internalwrapper/foam_dicts.py", line 192, in modifyNumerics controlDict) RuntimeError: Invalid arguments

TobiasRasp commented 8 years ago

Sorry, this was my mistake, please forget about my last comment!

khiltunen commented 8 years ago

Strange while travis is passed. That line 63 on foam_internalwrapper.py does not coontain call to modifyNumerics in the current push. Could you check that having latest updates.

TobiasRasp commented 8 years ago

Well, I mixed up different installations... Sorry for the confusion! The changes are all fine as far as I see and there's also an improvement in performance, especially for the internal wrapper.

But I found another point: In principal, iterative restarting of the OpenFoam-Wrapper works fine now for both the IO and the internal mode! But the time development is differing between the both. Let k > 0 be the iterator, then the time t in the IO wrapper evolves like t(k) = t(k-1) + N_dt with N being the number of timestep and dt the timestep. This is how it is expected. However, in the internal wrapper it evolves with t(k) = t(k-1) + k_N*dt.This leads to a differing Poiseulle profile development as shown in the image.

To me it seems like the simulation is started in each iteration from startTime = 0, but with an updated configuration (which also relates to the comment of santiagomarquezd from Jan 11th: "each calculation is like running from zero time"). This is suprising to me, since the internal controlDict generated in foam_dicts.py shows the correct entries with updated values of startTime: runTimeModifiable yes; startTime 0.0005; // which is t(k-1) in this case timeFormat general; writeCompression no; deltaT 5e-07; writePrecision 6; writeControl timeStep; application pimpleFoam; writeFormat ascii; startFrom startTime; writeInterval 10000; stopAt endTime; endTime 0.001; // t(k-1) + N*dt; purgeWrite 0;

Is this intended or a bug?

poisprof_internalio

khiltunen commented 8 years ago

At least it seems that the Time printed in pimpleFoam log is right also for concecutive runs. Could you send test code from which you have made that graph ?

TobiasRasp commented 8 years ago

Sure, please find the codes in the links! couple_CFDMD_internal.py.txt couple_CFDMD_io.py.txt

khiltunen commented 8 years ago

@TobiasRasp could you download test_mesh_dict.py also ?

TobiasRasp commented 8 years ago

Here it is test_mesh_dict.py.txt

khiltunen commented 8 years ago

@TobiasRasp i made modification to internal interface and now concecutive runs works also for internal wrapper. Also some modifications to mixture model data transfer were made.

TobiasRasp commented 8 years ago

@khiltunen Everything works fine now as far as I see. From my point of view this version is ready to merge!