-
Notifications
You must be signed in to change notification settings - Fork 17
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
Create interactive_guess.py #33
Conversation
Created tools folder and itneractive_fit.py; which contains (mainly) the classes InteractiveFit2D and InteractiveFit3D.
Added docstrings and some code comments for InteractiveFit2D and ProjectionPlot
# circumvented by creating a seperate callback function for each | ||
# parameter. | ||
for p in self.params: | ||
# p.value = self.sliders[p].val? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whats wrong with this line? Seems like the cleaner way to do it.
As a general remark, could you also include Could you add them to tests/tests_interactive.py? |
# I don't know (any more) if that's supported by self.model. | ||
vals = self.vec_func(self.xpoints, *list(p.value for p in self.params)) | ||
self.my_plot, = ax.plot(self.xpoints, vals, color='red') | ||
ax.scatter(self.xdata, self.ydata, c='blue') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.ydata -> self.dependent_data, which is still a dict
Revised doc-strings and InteractiveFit3D will follow.
Some minor changes in InteractiveFit2D docstrings. InteractiveFit3D changes are delayed to do something better than interpolating
Updated InteractiveFit2D to reflect (most of) tBuLi's comments. As for tests... I wouldn't even know where to start (A). But that may change of the course of a few days. |
So, to reduce the dimenionality in the data; some more ideas.
@tBuLi: Is it clear what I mean by this? And do you agree? |
All that remains for now is to reduce the dimensionality of the data at the set places.
Ok I looked into the math. For point 2 we agree, although I will show the maths I did to make sure we're doing the same thing. Where the integrals go over the domain we are interested in. Then: Gives the pdf of z. In order to plot z vs x, we can now plot the following: Where indeed this is <z(x)> after averaging out y. For point 1, the dataset, I think we could try repeating the procedure, replacing p(x, y) with the pdf of the kernel density. All that would be left is to figure out the conversion factor of the data back to the ámplitude' of the data. I'll think about that some more in the morning ;). Edit: wow, that tex2img service was a great succes |
and made some minor improvements to the tests.
plotrange_x = x_maxs[x.name] - x_mins[x.name] | ||
if not hasattr(self.model, 'initial'): | ||
x_mins[x.name] -= 0.1 * plotrange_x | ||
x_maxs[x.name] += 0.1 * plotrange_x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs comments. Why 0.1?
|
||
y_mins = {v: np.min(data) for v, data in self.dependent_data.items()} | ||
y_maxs = {v: np.max(data) for v, data in self.dependent_data.items()} | ||
for y in self.dependent_data: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs comments, what is this meant to do?
|
||
Parameters | ||
---------- | ||
n_points : int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong style of docstrings
|
||
for plotnr, proj in enumerate(self._projections, 1): | ||
x, y = proj | ||
if hasattr(self.model, 'dependent_derivatives') and Derivative(y, x) in self.model: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Derivative(y, x) in self.model
will suffice, right?
|
||
for plotnr, proj in enumerate(self._projections, 1): | ||
x, y = proj | ||
if hasattr(self.model, 'dependent_derivatives') and Derivative(y, x) in self.model: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Derivative(y, x) in self.model
suffices, right?
Thanks for the review. bae9f39 is ok if and only if Fit will always be a local optimization method. I have no problems with the rest of the changes. I'll process the rest of your comments later today. |
The cause for tBuLi#104 is still very much unknown though. Apparently adding the **2 here does the trick.
and brushed up some of the docstrings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the addition of a __str__
method, I will immediately merge with master. But I think that is a nice and simple addition.
|
||
guess = InteractiveGuess2D(model, A=concentration, t=tdata, n_points=250) | ||
|
||
print("Guessed values: ") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think InteractiveGuess objects should have __str__
defined such that when you call print(guess), we get essentially this report.
…d .execute to make the API consistant with the project
Created tools folder and interactive_fit.py; which contains (mainly) the classes InteractiveFit2D and InteractiveFit3D. These classes allow you to graphically create an initial guess for symfit, which is particularly valuable for complex and chaotic models.
Currently it is possible to use simple 2D models with InteractiveFit2D and multidimensional models with InteractiveFit3D. InteractiveFit2D should be extended to project the higher dimensions into multiple plots in 2 dimensions the same way InteractiveFit3D does in 3 dimensions.
@tBuLi: this was made some time ago, and works for symfit 0.2.6 on Python 2.7 and 3.4 (should anyway). I'm not sure the interal API changed/is used correctly/has impending changes coming.