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

Non-intutive behaviour for getp and setp #24

Closed
TorkelE opened this issue Jan 1, 2024 · 1 comment
Closed

Non-intutive behaviour for getp and setp #24

TorkelE opened this issue Jan 1, 2024 · 1 comment

Comments

@TorkelE
Copy link
Member

TorkelE commented Jan 1, 2024

So, with the latest update you cannot index structures directly for parameters, but instead have to use getp and setp. I understand that it make sense to split this into several steps for performance reasons, but stuff like:

getp(prob, k)(prob)

is both messy and confusing to the user. Furthermore, the performance cost (at least for small problems) is incredibly reliable.

Also, e.g. getp(oprob, p) really sounds like something that gets you the value of p in prob. Would it make sense to rename the current ones to something like:

k_getter = pgetter(prob, k)
k_setter = psetter(prob, k)

and then one can do

k_getter(prob)
k_setter(prob, 1.0)

Next, we could have getp and setp! which simply does what you want:

getp(prob, k)
setp(prob, k, 1.0)

something like this feels like a much more intuitive interface.

An alternative is the version where we permit indexing of prob.ps directly, so yo can do

prob.ps[k] # getp
prob.ps[k] = 1.0 # setp
@AayushSabharwal
Copy link
Member

Closing since SciML/SciMLBase.jl#584 released in https://github.com/SciML/SciMLBase.jl/releases/tag/v2.21.0 enables .ps[p] syntax

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