teokem / project-work-2018-MadsJeppesen

project-work-2018-MadsJeppesen created by GitHub Classroom
0 stars 1 forks source link

Project review by Albertas Dvirnas #1

Open albertasdvirnas opened 5 years ago

albertasdvirnas commented 5 years ago

This is a review of your report based on requirements on the course page https://github.com/mlund/jupyter-course as well as additional observations.

First of all the project seems to be very interesting and I would like to have time learn about it in more detail. That is not a purpose here though and I have only one comment about the project itself - when you fit all the distributions to the 6 parameters, it might not be entirely true that the distribution with the lowest SSE would be the best? (I am thinking here what would happen if some distributions have different number of fitting parameters, i.e. you have loc = params[-2], scale = params[-1], and then you might want to fix the set of distributions so that arg = params[:-2] has always the same length). Also, since the calculation for all distributions takes a long time, you could possibly save "best fit parameters" from the long calculation and after running an example for one distribution, just load them from the file.

From the project requirements, I found most of the things done well. Very nice documentation, and I was able to reproduce it on my computer.

Regarding the data manipulation you could save to disk the processed data with panda (i.e. the distribution parameters that you computed in best_fit_distribution.

The plots are well done and look very nice. I am not sure if figsize (9.5,4) provides figures 183 mm wide (they seem to be wider for me, but I could be misunderstanding how the image sizes work?), and these figures should be saved (there was a save figure button on the figures but it didn't seem to work properly for me)?

Thank you for making a very interesting report!

MadsJeppesen commented 5 years ago

Hi Albertas,

Thank you for the comments and useful feedback.

My comments:

I think you are right about saving the values for the best fit parameters and load them into the notebook instead. Here I'm also thinking in terms of reproducibility.

Yes, the figure size needs to be corrected according to what they describe - will do!

The save figure button should take you to a new tab and then you can save from there. But there might be something wrong. Nathan (the other reviewer) also had a problem with getting the backend to work properly. I'll see if there's something wrong that I can identify.

For the fitting. For this particular case I don't think you can change the amount of parameters you put in. But you are saying that to really "say" that the fit is the best you have to fit equal amounts of parameters for each? Or is it the evaluation using SSE that you think might not be good? Im not sure what exactly you mean - could you maybe elaborate?

Thanks again.

Best,

Mads