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

More robust conduit lengths #2853

Merged
merged 6 commits into from
Jan 26, 2024
Merged

More robust conduit lengths #2853

merged 6 commits into from
Jan 26, 2024

Conversation

jgostick
Copy link
Member

@jgostick jgostick commented Nov 3, 2023

The spheres_and_cylinders conduit length model was not handling an edge case properly. When two spheres overlapped, AND the throat was larger than the area of overlap, it was ignoring the contribution of the throat. It was basically assuming that the throat was much smaller than the pores.

I also added an extra check to this function so it raises an exception if the spheres overlap TOO much. We have had some people noticing negative flows and other odd behavior if they weren't being careful about spacing and pore sizes, so I decided to add this check even though we wanted to avoid too much "hand holding". I think this check is general enough that its acceptable.

Copy link

codecov bot commented Nov 3, 2023

Codecov Report

Merging #2853 (6d663d7) into dev (00551ab) will increase coverage by 0.0%.
Report is 2 commits behind head on dev.
The diff coverage is 100.0%.

Additional details and impacted files
@@          Coverage Diff          @@
##             dev   #2853   +/-   ##
=====================================
  Coverage   89.8%   89.8%           
=====================================
  Files        148     148           
  Lines       8624    8636   +12     
=====================================
+ Hits        7747    7759   +12     
  Misses       877     877           

@ma-sadeghi
Copy link
Member

ma-sadeghi commented Nov 3, 2023

@jgostick Should we instead add a health check to the run method? The very least, check if the conduit array (Nt by 3) is all positive.

@jgostick
Copy link
Member Author

jgostick commented Nov 4, 2023

I think it'd be a good idea to have a robust "check_physics_health" function that reports on errors. I'd rather not call it on each run though, since it slows things.

@jgostick jgostick merged commit 4d41843 into dev Jan 26, 2024
12 checks passed
@jgostick jgostick deleted the more_robust_conduit_lengths branch January 27, 2024 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants