search

stochasticHydroTools / libMobility

A collection of tools to compute hydrodynamic displacements.
MIT License
4 stars 0 forks source link

DPStokes parameter selection #19

Closed rykerfish closed 1 month ago

rykerfish commented 4 months ago

Here's the first pass at incorporating grid and kernel parameter selection into DPStokes. This would close #9. On first inspection, it seems to be working as the tests now go to 3 digits of decimal accuracy instead of just 2, but there might still be room to improve.

Current shortcomings:

  1. Only configured for one kernel
  2. Not a good way to test parameter selection
  3. No support for torques

Problem 1. is relatively easy to fix- I think we just take the kernel parameters as inputs and do a table lookup, then proceed as normal. We would need to include the error splines for the other kernels. The current error spline is somewhat grossly thrown at the bottom of mobility.h, so it might be nice to include those elsewhere in lookup files or something similar.

For problem 2, I thought it would be nice to set up tests on the parameter selection directly and compare to reference results that I could compute using the code from the DPStokes repository. However, the parameters don't seem easily accessible from the outside once they're set internally. To test, I think we'd have to change this (or someone let me know if this is possible and I just didn't figure it out!)

Finally for 3, this would be a thing to do later as there's some extra work involved in also setting grid/kernel parameters for torques, but I think the framework is there and it wouldn't be too difficult.

@brennansprinkle maybe we can talk sometime about what to do next?

RaulPPelaez commented 4 months ago

@rykerfish, if you write "Closes #9" at the end of your PR description, github will automatically close that when this PR is merged.

For 1: Either reintroduce some parameter like "w" into the interface or use "tolerance" with some heuristic to decide on w (which can be 4,5,6 AFAIK). For 2: Fo not test this, test the API directly like we are doing already, i.e "is the mobility correct?". If you write the parameter selection functions as free-standing you can snatch them in order to test them, but I do not think its worth the effort tbh. If the API gives you the correct mobilities then everything is working. For 3: When there is a PR introducing torques the tests for that will make evident what needs to be done.

RaulPPelaez commented 1 month ago

Is this PR obsolete now that #17 was merged?

rykerfish commented 1 month ago

Is this PR obsolete now that #17 was merged?

Definitely. It may be worth revisiting this topic at a later date to incorporate extensions like rectangular boxes or polydisperse particles, but at that point it'll just be worth starting over at wherever the main branch gets to in the meantime.