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

Add the function to update the pressure gradient in the hybrid solver #2110

Merged
merged 34 commits into from
May 14, 2024

Conversation

frankfeifan
Copy link
Contributor

In this PR, we have added a function in SinglePhaseHybridFVM to compute the cell-wise pressure gradient. Basically, the pressure gradient in a cell is computed according to the pressures on the cell center and all faces using a least squares method. So a routine to compute the least squares solution of a linear system is also added.

{
SinglePhaseBase::implicitStepComplete( time, dt, domain );

updatePressureGradient( domain );
Copy link
Contributor

Choose a reason for hiding this comment

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

If you plan on merging the PR with the call to BlasLapack in the kernel, maybe you can hard-code parallelHostPolicy in the kernel launch and add a flag to activate/skip the execute of updatePressureGradient from the XML file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've simply deleted updatePressureGradient here since it will only be called after one full load step in my phase-field PR

@CusiniM CusiniM added the flag: requires rebaseline Requires rebaseline branch in integratedTests label Sep 20, 2023
@CusiniM CusiniM added the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Sep 27, 2023
@paveltomin
Copy link
Contributor

@frankfeifan move into merge queue?

@frankfeifan
Copy link
Contributor Author

@frankfeifan move into merge queue?

I will have to merge the develop and rebaseline the tests, and then we can move it to merge queue.

@paveltomin
Copy link
Contributor

@frankfeifan move into merge queue?

I will have to merge the develop and rebaseline the tests, and then we can move it to merge queue.

thanks, merge to develop is one button on github click, right?
rebaseline does not make much sense before PR is on top of the queue (@CusiniM please correct me if i am wrong about that)

Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 51.00000% with 49 lines in your changes are missing coverage. Please review.

Project coverage is 53.57%. Comparing base (44684e1) to head (0c05252).

Files Patch % Lines
...sSolvers/fluidFlow/SinglePhaseHybridFVMKernels.hpp 0.00% 39 Missing ⚠️
.../physicsSolvers/fluidFlow/SinglePhaseHybridFVM.cpp 0.00% 8 Missing ⚠️
...nents/physicsSolvers/fluidFlow/SinglePhaseBase.hpp 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2110      +/-   ##
===========================================
- Coverage    53.58%   53.57%   -0.01%     
===========================================
  Files         1003     1003              
  Lines        85189    85289     +100     
===========================================
+ Hits         45645    45694      +49     
- Misses       39544    39595      +51     

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

@frankfeifan frankfeifan added the ci: run integrated tests Allows to run the integrated tests in GEOS CI label May 10, 2024
@CusiniM CusiniM merged commit a1fec52 into develop May 14, 2024
24 of 26 checks passed
@CusiniM CusiniM deleted the feature/frank/calculate_pressure_gradient branch May 14, 2024 02:56
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants