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

Break up multi fluid test #3140

Merged
merged 11 commits into from
Jun 11, 2024
Merged

Break up multi fluid test #3140

merged 11 commits into from
Jun 11, 2024

Conversation

dkachuma
Copy link
Contributor

@dkachuma dkachuma commented May 23, 2024

This PR breaks up the testMultiFluid unit test. This test does a full fluid update call and tests the obtained values of fluid properties against expected values. It also does numerical derivative checks at selected fluid conditions (pressure, temperature and composition).

This test was getting larger and taking longer to build especially with CUDA. Instead of having all the fluid types in one test, this is now broken up into different tests for the different multiphase fluid types. The common part is factored out into the MultiFluidTest test fixture.

The numerical derivative tests in this case were not active because a very large absolute tolerance was used (see #2876). These are now activated for most of the fluid types. As a consequence, a fix on the temperature derivative was required in EzrokhiBrineDensity.hpp. The tests are not activated for the CompositionalMultiphaseFluidPVTPackage because this already uses finite differences to calculate derivatives and for some of the test compositions, it is actually unable to give reliable values (the package itself needs updating).

The inclusion of testCompFlowUtils.hpp in constitutiveTestHelpers.hpp was pulling in too many include files into the compilation of thee tests. The methods required for the multiphase fluid tests have now been moved to a smaller include testFlowUtils.hpp.

This will likely require a rebaseline due to the change in the Ezrokhi density derivative.

@dkachuma dkachuma self-assigned this May 23, 2024
@dkachuma dkachuma added flag: requires rebaseline Requires rebaseline branch in integratedTests ci: run integrated tests Allows to run the integrated tests in GEOS CI ci: run CUDA builds Allows to triggers (costly) CUDA jobs type: cleanup / refactor Non-functional change (NFC) labels May 23, 2024
@dkachuma dkachuma marked this pull request as ready for review May 23, 2024 18:05
@dkachuma dkachuma requested a review from paveltomin May 23, 2024 18:06
@@ -194,11 +194,13 @@ void CO2BrineFluid< PHASE1, PHASE2, FLASH >::checkTablesParameters( real64 const
template< typename PHASE1, typename PHASE2, typename FLASH >
void CO2BrineFluid< PHASE1, PHASE2, FLASH >::initializePreSubGroups()
{
#if defined(GEOS_DEVICE_COMPILE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opposite or I am mssing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is supposed to be issued when it's a GPU run. The model is now activated for CPU only runs.

Copy link

codecov bot commented May 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.74%. Comparing base (010fc34) to head (b952b80).
Report is 101 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3140      +/-   ##
===========================================
- Coverage    53.77%   53.74%   -0.03%     
===========================================
  Files         1011     1016       +5     
  Lines        85816    85756      -60     
===========================================
- Hits         46146    46090      -56     
+ Misses       39670    39666       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dkachuma dkachuma mentioned this pull request Jun 4, 2024
24 tasks
@dkachuma
Copy link
Contributor Author

The fix to the derivative in EzrokhiBrineDensity has changed the response in class09_pb3_smoke_3d. The change in pressure is slightly different and quite expected. I would say a rebaseline is OK.

Error: /Problem/domain/MeshBodies/mesh1/meshLevels/Level0/ElementRegions/elementRegionsGroup/reservoir/elementSubRegions/1_hexahedra/deltaPressure
	Arrays of types float64 and float64 have 125 values of which 8 fail both the relative and absolute tests.
		Max absolute difference is at index (107,): value = 15333.86215468496, base_value = 15333.859636742622
		Max relative difference is at index (56,): value = 43.7011882327497, base_value = 43.70022329688072

@CusiniM CusiniM merged commit 4de1337 into develop Jun 11, 2024
26 checks passed
@CusiniM CusiniM deleted the refactor/dkachuma/fluid-tests branch June 11, 2024 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: requires rebaseline Requires rebaseline branch in integratedTests type: cleanup / refactor Non-functional change (NFC)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants