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

Handle kwargs and bounds in curve_fit #2545

Merged
merged 8 commits into from Apr 25, 2022
Merged

Conversation

jl-wynen
Copy link
Member

Fixes #2541

Comment on lines 66 to 71
if isinstance(b, Variable):
if b.shape != [2]:
raise ValueError("Parameter bounds must be either a tuple of scalar "
"variables or a 2-element array variable. "
f"Got sizes={b.sizes} as bounds for '{name}'.")
return tuple(b.to(unit=unit, dtype=float).values)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a bit odd to support this, I think? What is the meaning of the dimension?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None as far as curve_fit is concerned. I was just thinking it might be useful, similarly to passing limits as a length-2 array to sc.array.
But I don't mind removing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly to passing limits as a length-2 array to sc.array.

Not sure what you are referring to?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant sc.arange: #2140 (comment)

'a': sc.scalar(1.0, unit='s'),
'b': sc.scalar(1.0, unit='m')
},
bounds={'b': sc.array(dims=['xyz'], values=[-2.0, 2.0], unit='m')})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned above, we have an odd dummy dimension here?

@SimonHeybrock SimonHeybrock merged commit 7130317 into main Apr 25, 2022
@SimonHeybrock SimonHeybrock deleted the fix-keyword-args-curve-fit branch April 25, 2022 09:39
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

Successfully merging this pull request may close these issues.

curve_fit fails to forward kwargs to scipy
2 participants