You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
tracers is apparently expected only to be a tuple of symbols. Nevertheless, the function tracernames is called on this argument --- whose only purpose is to return tracer names when its argument may either be a tuple of symbols, or a tuple of fields.
The arguments (tracers, tracer_fields) and (timestepper_method, timestepper) are redundant.
Worse, tracer_fields can be set to something inconsistent with tracers, and therefore diffusivities and timestepper.
It's probably best if we assign only one keyword argument to each "concept", for the sake of simplicity and interpretability. A little bit of cleaning / interpretation to arguments (eg, if an argument may either be a tuple of symbols corresponding to tracer names, or a tuple of tracer fields) is ok, in my opinion, because it could make the code less confusing and easier to use.
The text was updated successfully, but these errors were encountered:
I guess part of the confusion is that some kwargs are for users while others were introduced for checkpointer usage. I agree that timestepper_method and timestepper are redundant. We should be able to just do something like time_stepper = AdamsBashforth2() for something.
Or go with your suggestion on Slack of having the choice of either passing a Symbol for a full struct. Users might usually pass tracers = (:a, :b) but checkpointer might pass tracers = TracerFields(...).
It's probably best if we assign only one keyword argument to each "concept", for the sake of simplicity and interpretability.
The signature of the model constructor is
Issues:
tracers
is apparently expected only to be a tuple of symbols. Nevertheless, the functiontracernames
is called on this argument --- whose only purpose is to return tracer names when its argument may either be a tuple of symbols, or a tuple of fields.The arguments (
tracers
,tracer_fields
) and (timestepper_method
,timestepper
) are redundant.Worse,
tracer_fields
can be set to something inconsistent withtracers
, and thereforediffusivities
andtimestepper
.It's probably best if we assign only one keyword argument to each "concept", for the sake of simplicity and interpretability. A little bit of cleaning / interpretation to arguments (eg, if an argument may either be a tuple of symbols corresponding to tracer names, or a tuple of tracer fields) is ok, in my opinion, because it could make the code less confusing and easier to use.
The text was updated successfully, but these errors were encountered: