resolves 1912: implement nirb tools #1944

thomas-saigre · 2022-09-21T09:30:25Z

Have you checked to ensure there aren't other open Pull Requests for the same change?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes?
Have you successfully run the Feel++ testsuite with your changes locally?
Have you written Doxygen comments in your contribution ?

This is not working in parallel yet

close implement nirb framework for elliptic problems #1912

[skip feelpp skip tests skip toolboxes skip mor]

- define once element space Xh and interpolator - fix docstrings

…into 1912-implement-nirb-tools

…b-tools

! parallel tests are commented in the file [skip feelpp skip tests skip toolboxes skip mor]

…b-tools

…skip mor

…s skip mor

mor/cases/thermal-fin/3d/thermal-fin.cfg

@vincentchabannes

/cc @vincentchabannes [ci skip]

thomas-saigre · 2023-02-07T15:57:49Z

.vscode/settings.json

    },
-    "C_Cpp.configurationWarnings": "Disabled",
+    "jupyter.notebookFileRoot": "${workspaceFolder}/build/python/install/lib/python3/dist-packages",


@prudhomm @vincentchabannes @alielarif I'm not sure this line should stay here : if the preset selected is not python there is no reason this path should be correct.
The advantage of this setting it that when we run a notebook in vscode, it automatically find the development packages installed in build directory, so there is no need to set by hand $PYTHONPATH.
The drawback is that when we launch a kernel, the working directory is no longer the directory where the ipynb file is saved, it is the one set in this line. So it is messy to load the config file (for example in python/pyfeelpp-mor/feelpp/mor/nirb/test_nirb_offline.ipynb, the path is hard coded to a local path "/home/saigre/Documents/code/feelpp/python/pyfeelpp-mor/feelpp/mor/nirb/model/square/square.cfg" which should have not been pushed, my bad !)

I don't know what would be the best way to set the environment easily : automatically set PYTHONPATH to the install directory, while the working directory still being the one where the notebook file is saved...

could you set this in your user settings and not the workspace setting ?

prudhomm · 2023-02-27T07:19:17Z

.vscode/settings.json

    },
-    "C_Cpp.configurationWarnings": "Disabled",
+    "jupyter.notebookFileRoot": "${workspaceFolder}/build/python/install/lib/python3/dist-packages",


could you set this in your user settings and not the workspace setting ?

prudhomm · 2023-02-27T07:19:33Z

.vscode/settings.json

+    "cmake.parallelJobs": 5,
+    "autoDocstring.docstringFormat": "numpy"


I looked at the list available for default docstring format. When the default one is use, the overview of it in vscode is given in the first figure, with numpy, the format is slightly different and the overview is given in the second figure.
I find this is better the second way, but of course we can choose something else !

prudhomm · 2023-02-27T07:21:39Z

python/pyfeelpp-mor/feelpp/mor/nirb/nirb.py

+        if self.toolboxType == "heat":
+            return toolbox.fieldTemperature()
+        elif self.toolboxType == "fluid":
+            return toolbox.fieldVelocity()
+        else:
+            raise ValueError("Unknown toolbox")


how about using a dict to map the function calls ? that way it extends in a nicer way to other toolboxes

prudhomm · 2023-02-27T07:23:03Z

python/pyfeelpp-mor/feelpp/mor/nirb/nirb.py

+        if self.toolboxType == "heat":
+            tb = heat.heat(dim=self.dimension, order=self.order)
+        elif self.toolboxType == "fluid":
+            tb = fluid.fluid(dim=self.dimension)
+        else:
+            raise ValueError("Unknown toolbox")


how about create the toolbox outside ToolboxModel ?

This will lead to a lot of code changes. Better to do it in an other branch.

prudhomm · 2023-02-27T07:23:43Z

python/pyfeelpp-mor/feelpp/mor/nirb/nirb.py

+        if self.toolboxType == "heat":
+            Vh_image = image_tb.spaceTemperature()
+            Vh_domain = domain_tb.spaceTemperature()
+        elif self.toolboxType == "fluid":
+            Vh_image = image_tb.spaceVelocity()
+            Vh_domain = domain_tb.spaceVelocity()


this is not good I think to have this everywhere, if we have to extend this to other toolboxes

A facade needs to be created : please follow this: https://www.geeksforgeeks.org/facade-method-python-design-patterns/

we need to discuss this before doing any major changes to the code

prudhomm · 2023-02-27T07:27:17Z

python/pyfeelpp-mor/feelpp/mor/nirb/nirb.py

+            Bh = self.coeffFine[:nb, :nb]
+            R = np.zeros((nb,nb))
+            for s in range(nb):
+                R[s,:] = (np.linalg.inv(BH.transpose() @ BH + regulParam*np.eye(nb))@BH.transpose()@Bh[:,s])


avoid calling .inv, rather call .solve with multiple right hand sides, @thomas-saigre

Yes the correction was made in another branch (in commit 1991f58 on branch 2038-nirb-greedy), we may cherry-pick this commit to put the correction in here

prudhomm · 2023-02-27T07:30:17Z

python/pyfeelpp-mor/feelpp/mor/nirb/test_process.py

+
+    import sys 
+
+    dataPath = "/data/home/elarif/feelppdb/nirb/heat/np_1/results/Rect/"


this should not be hardcoded, please use feel++ framework and repositories

we replace this file by a notebook one in the branch #2041. This will be easy to manipulate.

prudhomm · 2023-02-27T07:32:00Z

python/pyfeelpp-mor/tests/test_nirb.py

+    feelpp.Environment.setConfigFile(casefile)
+
+    run_offline(model_path, rect, greedy)
+    run_online(model_path, rect)


are there any verifications that the results are ok ?

prudhomm · 2023-02-27T07:34:40Z

@alielarif @thomas-saigre minor changes are required.
for example remove all hardcoded path and use feelpp repository handling instead

thomas-saigre · 2023-03-02T10:34:15Z

.vscode/settings.json

I think I have pussed this by error : I think it was 25 before (too much for my personal laptop), I thought I updated this value only locally, but it seems that it commited anyway, sorry about that.
Shall we change it back ?

alielarif · 2023-03-07T11:50:41Z

python/pyfeelpp-mor/tests/test_nirb.py

+        #  (('testcase/nirb/lid-driven-cavity/', 'cfd2d.cfg', 'cfd2d.json', True) , 'lid-driven-cavity rect'),
+         (('testcase/nirb/square', 'square.cfg', 'square.json', False, False), 'square2d w/o rect wogreedy'),
+         (('testcase/nirb/square', 'square.cfg', 'square.json', True, False) , 'square2d rect wogreedy'),
+         (('testcase/nirb/square', 'square.cfg', 'square.json', True, True) , 'square2d rect egreedy'),


the test with Greedy selection dosn't get an error less than 8.10-2. I remove it now by waiting its new version.

…into 1912-implement-nirb-tools

…xes skip mor

[ci skip]

Ali Elarif and others added 25 commits September 16, 2022 17:03

up nirb library

0695584

fix initialisation of the environment

b99f8d8

refactor code :

db530e5

- define once element space Xh and interpolator - fix docstrings

up test to make online measurments

1da15ef

remove selection of eigen values

64908ac

fix orthonormalization, add computing of interp error

7ccb773

up perf test

fad3b50

solve merge conflicts

55607c0

use time() to evaluate elapsed time

f2ea13a

compute mean error of a sampling of parameters

74363b1

set exporter outside of online function

4bd05fb

rename method getSolution -> getToolboxSolution

f3829ec

fix docstrings

0c867fa

rewrite some petsc matrix for parallel solving

386074b

solve merge conflicts

b1601fb

up 3d example

2272d6e

Merge branch '1912-implement-nirb-tools' of github.com:feelpp/feelpp …

c6ed111

…into 1912-implement-nirb-tools

Optimize code : remove some storage on nirbOnline class

503d32a

add functions for computing nirb errors

01aeda0

add k0 parameter in the json file

51ec45a

Merge branch from remote to local

fe90eb7

refactor : move functional space in class ToolboxModel

fa075d3

Merge branch '1912-implement-nirb-tools' of github.com:feelpp/feelpp …

42aa29b

…into 1912-implement-nirb-tools

Merge remote-tracking branch 'origin/develop' into 1912-implement-nir…

9baca37

…b-tools

up test_nirb to test suite

134606d

! parallel tests are commented in the file [skip feelpp skip tests skip toolboxes skip mor]

thomas-saigre requested review from prudhomm and vincentchabannes as code owners September 21, 2022 09:30

thomas-saigre linked an issue Sep 21, 2022 that may be closed by this pull request

implement nirb framework for elliptic problems #1912

Closed

9 tasks

thomas-saigre and others added 2 commits September 21, 2022 11:32

fix tests in python

376aab0

fix parallel execution in PODreducedBasis

592e885

Ali Elarif added 4 commits January 30, 2023 14:53

get offline infos in additions to toolbox infos.

7dd872a

up online and offline exec

f73d811

up online Lagrangian norm skip feelpp skip tests skip toolboxes skip mor

b8bd118

Merge remote-tracking branch 'origin/develop' into 1912-implement-nir…

8fd3ddc

…b-tools

alielarif requested review from vincentchabannes and prudhomm January 30, 2023 15:56

Ali Elarif added 5 commits January 31, 2023 14:33

fix call of online normMat

3fe7904

comment possible cause of a bug

2cfb374

write sample by only one proc. skip feelpp skip tests skip toolboxes …

1022508

…skip mor

fix loading 'contactforce.hpp'

fe8aa1b

check the size of loaded sample. skip feelpp skip tests skip toolboxe…

4de9a00

…s skip mor

vincentchabannes approved these changes Feb 1, 2023

View reviewed changes

mor/cases/thermal-fin/3d/thermal-fin.cfg Outdated Show resolved Hide resolved

discard "journal=false" in config file

e82a120

/cc @vincentchabannes [ci skip]

thomas-saigre commented Feb 7, 2023

View reviewed changes

prudhomm mentioned this pull request Feb 18, 2023

setup jupyter notebook root folder #2044

Closed

1 task

Merge branch 'develop' into 1912-implement-nirb-tools

c560758

prudhomm requested changes Feb 27, 2023

View reviewed changes

thomas-saigre commented Mar 2, 2023

View reviewed changes

alielarif reviewed Mar 7, 2023

View reviewed changes

alielarif and others added 4 commits March 7, 2023 13:03

add checking results

3344d10

Merge branch '1912-implement-nirb-tools' of github.com:feelpp/feelpp …

38b46fd

…into 1912-implement-nirb-tools

rename files and remove hardcoded. skip feelpp skip tests skip toolbo…

0c58047

…xes skip mor

refactor : use np.linalg.solve instead of a for loop

8dd882e

[ci skip]

alielarif requested a review from prudhomm March 8, 2023 13:22

prudhomm approved these changes Mar 8, 2023

View reviewed changes

prudhomm merged commit 186642b into develop Mar 8, 2023

prudhomm deleted the 1912-implement-nirb-tools branch March 20, 2023 08:12

prudhomm added the ✨ type:feature label Mar 18, 2024

prudhomm changed the title ~~1912-implement-nirb-tools~~ resolves 1912: implement nirb tools Mar 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resolves 1912: implement nirb tools #1944

resolves 1912: implement nirb tools #1944

thomas-saigre commented Sep 21, 2022 •

edited by prudhomm

Loading

thomas-saigre Feb 7, 2023 •

edited

Loading

prudhomm Feb 27, 2023

prudhomm Feb 27, 2023

prudhomm Feb 27, 2023

thomas-saigre Mar 2, 2023

prudhomm Feb 27, 2023

prudhomm Feb 27, 2023

alielarif Mar 7, 2023

prudhomm Feb 27, 2023

prudhomm Feb 27, 2023

prudhomm Feb 27, 2023

prudhomm Feb 27, 2023

thomas-saigre Mar 2, 2023

prudhomm Feb 27, 2023

alielarif Mar 7, 2023

prudhomm Feb 27, 2023

prudhomm commented Feb 27, 2023

thomas-saigre Mar 2, 2023

alielarif Mar 7, 2023

		"cmake.parallelJobs": 5,
		"autoDocstring.docstringFormat": "numpy"


		import sys

		dataPath = "/data/home/elarif/feelppdb/nirb/heat/np_1/results/Rect/"

resolves 1912: implement nirb tools #1944

resolves 1912: implement nirb tools #1944

Conversation

thomas-saigre commented Sep 21, 2022 • edited by prudhomm Loading

thomas-saigre Feb 7, 2023 • edited Loading

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

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

prudhomm commented Feb 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomas-saigre commented Sep 21, 2022 •

edited by prudhomm

Loading

thomas-saigre Feb 7, 2023 •

edited

Loading