mesh-adaptation/goalie

Don't use `get_form`

Opened this issue · 3 comments

ddundo commented 
          Note: I think it would make sense to get rid of `get_form` since it's only used in `indicate_errors` and just combine it with `get_solver`. This would avoid complications with communicating time-dependent fields between `get_solver` and `get_form` inside the user code. We'd still need a way to communicate the form to goalie, though - could simply yield it, introduce some new `MeshSeq` method for it...

Originally posted by @ddundo in #73 (comment)

This would also simplify user code. What do you think @jwallwork23?

jwallwork23 commented

Yeah I agree we can get rid of it.

  • 👍1
acse-ej321 commented

@ddundo - if you yield {'<field>': F} from the solver and pass it through the _solve_adjoint function to indicate_errors, you probably don't need a new method? - just catch and release of the yielded variable.

ddundo commented

Thanks @acse-ej321! Yeah, in the above comment I suggested either yielding it or adding some method like MeshSeq.set_form(). I think both are fine and it's just a matter of taste maybe. I think I'd prefer the method option because there's not really a reason to yield the form at every timestep, since

  • the coefficients get updated automatically
  • we'd only pass the last one to indicate_errors
  • I'd prefer to keep things in the _solve_adjoint code tidier (as it's already quite long)
  • to make this communication between user and goalie code a bit more explicit, which also helps clarity I think

For the same reason I didn't do yield field but rather read them from MeshSeq.fields, where they're automatically updated.

But yeah, not sure what's best! I'll open a PR and then it will be easier to discuss :)