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

Should pore.rate be automatically included on the algorithm? #2720

Open
jgostick opened this issue Mar 31, 2023 · 3 comments
Open

Should pore.rate be automatically included on the algorithm? #2720

jgostick opened this issue Mar 31, 2023 · 3 comments

Comments

@jgostick
Copy link
Member

Questions like #2719 comes up often enough that maybe we should change how we do this? Instead of having users to alg.rate(pores=Ps), they'd do alg['pore.rate'][Ps].sum()?

Of course, we'd have to support both, but could transition to the latter approach slowly. It's definitely in keeping with our desire to keep things simple and put power into the user's hands.

@jgostick
Copy link
Member Author

jgostick commented Apr 3, 2023

Regarding the question of how often to update rate, I think that if you update conductance, then the 'pore.quantity' values are wrong but we had no problem with that. So the rate value should be computed each time the quantity is computed.

Another thing that needs to be considered is how to deal with transient simulations..should we store the rate at each time like we do with quantity?

@ma-sadeghi
Copy link
Member

I see your point. There's another issue: if we were to update pore.rate whenever we update pore.quantity, then we need to "make sure" that we do this for all algorithms (and those that gets added in the future), which is another entry point for bugs: e.g., how to deal with multiphysics algs, transient algs, etc. To put it another way, since pore.rate is not a necessary quantity to be calculated, it makes sense for it to be calculated on demand, whereas pore.quantity is a bit different, in that it's most of the time the only thing that you want from a simulation.

Although given that we won't be adding too many algorithms to openpnm at this stage, this might not be a real concern.

@jgostick
Copy link
Member Author

jgostick commented Jun 7, 2023

Hmmm, 'pore.rate' and 'throat.rate' could be models? Then they would potentially be kept up to date when other things are changed? We can add models to algorithms now after all.

@ma-sadeghi ma-sadeghi changed the title Should 'pore.rate' be automatically included on the algorithm? Should pore.rate be automatically included on the algorithm? Mar 8, 2024
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

No branches or pull requests

2 participants