Black-Oil fluid model integration #278

klevzoff · 2018-12-14T07:18:38Z

Work on getting PVTPackage black-oil model running and tested

GEOS-DEV/PVTPackage#12

…est case updates for black-oil.

…lack-oil

…h paths to table files.

…luid unit tests.

klevzoff · 2018-12-15T01:30:49Z

src/coreComponents/constitutive/Fluid/BlackOilFluid.cpp

@@ -137,6 +139,29 @@ void BlackOilFluid::createFluid()
  std::vector<double> densities( m_surfaceDensities.begin(), m_surfaceDensities.end() );
  std::vector<double> molarWeights( m_componentMolarWeight.begin(), m_componentMolarWeight.end() );

+  // if table file names are not absolute paths, convert them to such, based on path to main input/restart file
+  ProblemManager const * const problemManager = this->GetGroupByPath<ProblemManager>("/");


@rrsettgast This thing here is a but ugly... let me know if you have a suggestion (see the code below).

Basically, the black-oil interface of PVTPackage consumes the file names of black-oil table files and reads those internally. The input xml will typically specify relative paths to these files, but we have to provide full paths, since there is no guarantee the current working directory is the one containing the xml.

I worked around this by taking the input/restart file name (which I think is always absolute) from the ProblemManager, extracting the path component and slapping the relative path to the file onto it. What I don't like is the additional dependency of the fluid model on ProblemManager. This has already caused some trouble in the fluid unit test (which does not have a ProblemManager as root, nor an xml input file). I had to resort to writing the tables into temporary files, which are accessed by relative path, and deleting them afterwards. I wonder if there is a better way.

Sorry. Perhaps I don't understand completely, but is it possible to get a full path from a relative path when reading in the string that contains the relative path?

This would be great but it is c++17
https://en.cppreference.com/w/cpp/filesystem/absolute

This might work:
https://stackoverflow.com/questions/27247991/how-to-get-full-path-by-just-giving-filename
https://stackoverflow.com/questions/1661982/how-do-i-get-the-full-path-for-a-filename-command-line-argument

@rrsettgast to clarify what I meant: let's say you have a directory structure (simplified):

GEOSX | +-- build | | | +-- bin | | | +-- geosx | +-- tests | | | +-- blackoil.xml | | | +-- pvto.txt ...

where blackoil.xml contains a reference to pvto.txt (short name, just like that) and you run as follows:

~/GEOSX$ cd build/bin ~/GEOSX/build/bin$ ./geosx -i ../../tests/blackoil.xml

Then in the code you cannot open that file by doing fopen("pvto.txt"), nor can you do realpath("pvto.txt"), since in both cases the system will attempt to interpret it relative to the working dir, which is ~/GEOSX/build/bin.

What I ended up doing is taking the input file name (../../tests/blackoil.xml which is expanded into e.g. /home/klevtsov/GEOSX/tests/blackoil.xml) from ProblemManager, extracting the path component (/home/klevtsov/GEOSX/tests/) and then adding pvto.txt to it to give me an actual full path to the file.

What I don't like is the added dependency of the fluid model on ProblemManager, which for instance makes unit testing a bit more cumbersome. But I guess there may not be a way around it: you have to get the path somewhere.

One thing to try would be to add a path into the xmlWrapper object, set it when the xml file is opened, and then use it to set the absolute paths when you need it. The path would have to be a static member that you set manually, but at least that would save you from the inverse dependency.

Or we could make a "path" type that gets converted to an absolute path when read from the xml.

… from PVTPackage. Small improvements to Newton and line search loops.

klevzoff · 2018-12-19T01:08:17Z

src/coreComponents/managers/ProblemManager.cpp

@@ -54,6 +56,43 @@ using namespace dataRepository;
 using namespace constitutive;


+struct Arg : public option::Arg


There was no reason for this to be in ProblemManager.hpp, was there? As soon as I included it in BlackOulFluid.cpp, I was forced to link my unit tests with optionparser. Instead I opted to move these into the cpp, doesn't seem to break anything.

I can't think of one. Perhaps it should be in its own file. @cssherman, this was your addition. Any objections?

klevzoff · 2018-12-19T01:14:41Z

src/coreComponents/physicsSolvers/SolverBase.cpp

+    }
+    else
+    {
+      GEOS_ERROR( "Nonconverged solutions not allowed. Terminating..." );


Previously we would simply proceed to the next time step without advancing the state of the physical domain. I think this makes more sense, we either accept whatever solution available to at least remain physically consistent with time (although likely inaccurate), or completely bail out.

Yes. We can add in something to do a reduced timestep when that is supported.

klevzoff · 2018-12-19T01:15:55Z

src/coreComponents/physicsSolvers/SolverBase.cpp

+void SolverBase::CreateChild( string const & childKey, string const & childName )
+{
+  // recognize SystemSolverParameters, the group is already registered
+  if (childKey == groupKeyStruct::systemSolverParametersString)


This fixes the child not recongnized message that pops up every time

klevzoff · 2018-12-19T01:20:17Z

src/coreComponents/constitutive/unitTests/testMultiFluid.cpp

+  {
+    writeTableToFile( "pvto.txt", pvto_str );
+    writeTableToFile( "pvtg.txt", pvtg_str );
+    writeTableToFile( "pvtw.txt", pvtw_str );


This test writes Black-Oil PVT tables to temporary files during setup and cleans up files at tear down. This is a better (more self-contained) approach than keeping the files in repo and trying to pass the path to them.

klevzoff · 2018-12-19T01:31:34Z

So I'm gonna wrap this up for now and would like to merge. The interface to Black-oil fluid model is fully functional, but the test cases I've tried have some issues with Newton convergence after a number of time steps, hence no integrated test at the moment. It might be related to derivative discontinuities in the fluid model, but I think it calls for a separate investigation.

@rrsettgast can you approve or suggest someone for a review?

rrsettgast · 2018-12-24T06:27:50Z

src/coreComponents/constitutive/Fluid/BlackOilFluid.cpp

@@ -137,6 +139,29 @@ void BlackOilFluid::createFluid()
  std::vector<double> densities( m_surfaceDensities.begin(), m_surfaceDensities.end() );
  std::vector<double> molarWeights( m_componentMolarWeight.begin(), m_componentMolarWeight.end() );

+  // if table file names are not absolute paths, convert them to such, based on path to main input/restart file
+  ProblemManager const * const problemManager = this->GetGroupByPath<ProblemManager>("/");


Sorry. Perhaps I don't understand completely, but is it possible to get a full path from a relative path when reading in the string that contains the relative path?

This would be great but it is c++17
https://en.cppreference.com/w/cpp/filesystem/absolute

This might work:
https://stackoverflow.com/questions/27247991/how-to-get-full-path-by-just-giving-filename
https://stackoverflow.com/questions/1661982/how-do-i-get-the-full-path-for-a-filename-command-line-argument

rrsettgast · 2018-12-24T06:30:23Z

src/coreComponents/constitutive/RelPerm/BrooksCoreyRelativePermeability.cpp

+    }
+    else
+    {
+      relPerm[ip] = (satScaled < 0.0) ? 0.0 : scale;


Sorry....is satScaled>1 being handled as desired here?

The goal is to just assign kr_max (labeled scale here) whenever the argument (scaled saturation) goes above 1... I think this is what it does. The name scale is confusing here, I should've named the variable maxValue.

rrsettgast · 2018-12-24T06:34:22Z

src/coreComponents/physicsSolvers/FiniteVolume/CompositionalMultiphaseFlow.cpp

@@ -606,7 +618,7 @@ void CompositionalMultiphaseFlow::UpdateComponentFraction( ManagedGroup * const
  arrayView3d<real64> const & dCompFrac_dCompDens =
    dataGroup->getReference<array3d<real64>>( viewKeyStruct::dGlobalCompFraction_dGlobalCompDensityString );

-  forall_in_range( 0, dataGroup->size(), GEOSX_LAMBDA ( localIndex const a ) mutable
+  forall_in_range( 0, dataGroup->size(), GEOSX_LAMBDA ( localIndex const a )


klevzoff added 4 commits December 13, 2018 21:28

Support reading of black-oil tables from files. Minor bug fixes and t…

8efc429

…est case updates for black-oil.

Merge remote-tracking branch 'origin/develop' into feature/klevzoff/b…

3a664e7

…lack-oil

Add some consistency checks between fluid and relperm models

47fcf71

Add unit tests for black-oil fluid model. There is still an issue wit…

1ace63a

…h paths to table files.

klevzoff self-assigned this Dec 14, 2018

klevzoff added type: feature New feature or request priority: high labels Dec 14, 2018

Resolve issues with table file paths. Improve the structure of MultiF…

0b96a15

…luid unit tests.

klevzoff commented Dec 15, 2018

View reviewed changes

klevzoff added 4 commits December 14, 2018 17:35

Remove unused black-oil table files in fluid unit test

61645e3

Fix relperm models. Add a 10-cell BO test case. Suppress debug output…

cc4c6fc

… from PVTPackage. Small improvements to Newton and line search loops.

Minor updates. Shortcut to avoid computing immobile phase flux.

c21cd0f

Remove mutable from kernel lambdas

9987903

klevzoff commented Dec 19, 2018

View reviewed changes

klevzoff requested review from rrsettgast and cssherman December 19, 2018 01:26

rrsettgast approved these changes Dec 24, 2018

View reviewed changes

klevzoff merged commit e89d83a into develop Dec 28, 2018

klevzoff deleted the feature/klevzoff/black-oil branch February 14, 2019 22:28

This was referenced Dec 1, 2023

Fixes for brine density computes, output internally constructed PVT tables for CO2 fluid into CSV files #2792

Merged

Some checks in testMultiFluid are effectively inactive #2876

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Black-Oil fluid model integration #278

Black-Oil fluid model integration #278

klevzoff commented Dec 14, 2018 •

edited

Loading

klevzoff Dec 15, 2018 •

edited

Loading

rrsettgast Dec 24, 2018

klevzoff Dec 24, 2018 •

edited

Loading

rrsettgast Dec 24, 2018

klevzoff Dec 19, 2018 •

edited

Loading

rrsettgast Dec 19, 2018

klevzoff Dec 19, 2018

rrsettgast Dec 19, 2018

klevzoff Dec 19, 2018

rrsettgast Dec 19, 2018

klevzoff Dec 19, 2018

klevzoff commented Dec 19, 2018

rrsettgast Dec 24, 2018

rrsettgast Dec 24, 2018

klevzoff Dec 24, 2018

rrsettgast Dec 24, 2018

		@@ -54,6 +56,43 @@ using namespace dataRepository;
		using namespace constitutive;


		struct Arg : public option::Arg

Black-Oil fluid model integration #278

Black-Oil fluid model integration #278

Conversation

klevzoff commented Dec 14, 2018 • edited Loading

klevzoff Dec 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klevzoff Dec 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klevzoff Dec 19, 2018 • 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

klevzoff commented Dec 19, 2018

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

klevzoff commented Dec 14, 2018 •

edited

Loading

klevzoff Dec 15, 2018 •

edited

Loading

klevzoff Dec 24, 2018 •

edited

Loading

klevzoff Dec 19, 2018 •

edited

Loading