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

Parameter passing to apply() #6

Closed
jhueckelheim opened this issue May 19, 2017 · 1 comment
Closed

Parameter passing to apply() #6

jhueckelheim opened this issue May 19, 2017 · 1 comment

Comments

@jhueckelheim
Copy link

jhueckelheim commented May 19, 2017

The apply_forward() and apply_reverse() methods take no arguments. This should probably be changed, and arguments be passed on to the operator.apply() methods. Before this is implemented, we need to discuss the desired behaviour. Complications:

  • apply_reverse() calls fwd_operator.apply() and rev_operator.apply() internally, several times. Do we need to distinguish which arguments to apply_reverse() are passed to which operator?
  • The operator apply routines take t_start and t_end as arguments. These are now set internally by pyrevolve. If a user mis-uses pyrevolve and provides such arguments to apply_reverse() or apply_forward(), we can actively discard the user-provided arguments, throw an error, or blindly forward the user's values and potentially compute wrong results.
  • A user may try to pass symbols to apply_reverse() that contain data that was modified after running apply_forward(), perhaps in a misguided attempt to compute several adjoint result sets. This would again silently produce wrong results, as the new data provided will be inconsistent with the data inside the checkpoint storage. Do we want to sanity-check things like this, or assume that the user is always right?

The easiest way out (for pyrevolve) is of course to just pass everything without checks, and crash and burn if the user is stupid.

@navjotk
Copy link
Member

navjotk commented Jul 20, 2020

I think this is handled well by storing the arguments on the Operator wrapper object as shown here

@navjotk navjotk closed this as completed Jul 20, 2020
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