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

fix: Add HU simplified 2-phase version, refactor IHU implementation and fix bugs where possible, change phase/component flux logic #3401

Open
wants to merge 100 commits into
base: develop
Choose a base branch
from

Conversation

paveltomin
Copy link
Contributor

@paveltomin paveltomin commented Oct 16, 2024

  1. Another implementation of for hybrid upwinding, limited to 2 phases, tries to do total mobility treatment closer to published schemes
  2. Refactored IHU implementation, added few helper functions to minimize code duplication, fixed/attempted to fix the following bugs:
  • densMean * dGravD_dP should be outside of for( localIndex j = 0; j < numFluxSupportPoints; ++j ) subloop (not very important since dTrans_dPres is often zero), similar fix in capillary flux
  • 'totMob += phaseMob[seri[ke]][sesri[ke]][sei[ke]][jp]' and derivatives should not be in the for( localIndex ke = 0; ke < numFluxSupportPoints; ++ke ) subloop - but i could not change this one without getting bad convergence for barrier test case - results with wrong treatment are definitely off but i am not sure what to do with that, @jafranc any thoughts?
  • derivatives for total mobility here: dFractionalFlow_dP[ke] -= fractionalFlow * dTotMob_dP[k_up_ppu] / LvArray::math::max( totMob, minTotMob ) should be taken from ke not k_up_ppu (same in other similar places)
  1. Change phase/component flux logic - assemble full phase flux first and then do x_cp upwinding once to compute component flux. The danger of old treatment is that phaseFlux being used in subsequent computations and it might be tricky to separate parts of phase flux to compute component flux

Base automatically changed from pt/flow-kernel-comp-split to develop November 11, 2024 00:46
@paveltomin paveltomin added type: bug Something isn't working type: feature New feature or request labels Nov 12, 2024
@paveltomin paveltomin changed the title feat: Add HU simplified 2-phase version, refactor IHU implementation and fix bugs where possible, change phase/component flux logic fix: Add HU simplified 2-phase version, refactor IHU implementation and fix bugs where possible, change phase/component flux logic Nov 12, 2024
@paveltomin
Copy link
Contributor Author

Waiting on code owner review from @CusiniM, @castelletto1, @rrsettgast, and/or @ryar9534.

@paveltomin
Copy link
Contributor Author

@CusiniM, @castelletto1, @rrsettgast, and/or @ryar9534 please review

@paveltomin paveltomin added flag: requires rebaseline Requires rebaseline branch in integratedTests and removed ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Dec 2, 2024
@paveltomin paveltomin added the ci: run integrated tests Allows to run the integrated tests in GEOS CI label Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: ready for review flag: requires rebaseline Requires rebaseline branch in integratedTests type: bug Something isn't working type: feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants