Permit pgen-defined custom source terms #30
Conversation
I think I'd prefer to not push the isolated planet setup. Adam and I are discussing other pgen setups that better exploit some of |
It seems to me that we want to provide a general problem-specific source function? It would just be nice to combine it with a use case so that it is, like, compiled during the CI to check that it works. Maybe we don't have an immediate idea of when such a source for a problem will be needed by us. |
I agree it would be nice to have a working example to go with this change. I'm sure we can think of some minor source function for one of the existing test problems. Like maybe in the conduction problem, you add some heat source/sink. Or you implement some special damping function in the disk problem. Doesn't have to be something serious. But I don't feel strongly about needing something so long as you've tested separately that this works |
I think this feature might be a nicety for some folks and is a commonly provided utility in other astro-codes. Luckily the CI will compile the new task and its execution (but it will always trigger the I feel goofy adding in a no-op source term to a pgen for the sole purpose of checking that the other pathway triggers correctly. Maybe we need a unique pgen dedicated to testing every possible custom enrollment, but it isn't actually a true pgen, moreso just a unit test --- but this is out-of-scope for this PR. |
There was a problem hiding this comment.
LGTM, good feature to have. Any objections to merge @adamdempsey90?
| template <Coordinates GEOM> | ||
| TaskStatus ProblemGeneratorSourceTerm(MeshData<Real> *md, const Real time, | ||
| const Real dt) { | ||
| PARTHENON_FAIL("Disk pgen-defined source term not yet implemented!"); | ||
| return TaskStatus::complete; | ||
| } |
There was a problem hiding this comment.
Is there a reason why you put a stub in this pgen and not all others? I'm not sure it's needed since it's clear from the std::function in problem_modifier what the signature is.
There was a problem hiding this comment.
Just as a demonstration. Notice that we also use the disk pgen to hold a stub for user-defined refinement criteria.
I would still like to see a modification of one of the existing tests with a super simple source term. @pdmullen is that out of the question right now? |
This is a totally reasonable request. But I think the "right" solution here is introducing a |
I don't see the rush in merging this, so I'm fine with waiting until the CI covers this new stuff. You could add the user defined refinement criteria to your new test too ;) |
Background
Permits custom pgen-defined source terms.
Description of Changes
Adds logic to enroll a custom pgen source term. The task is called independent of enrollment, but checks for
nullptr.Checklist