Ale0430 / MiniBuoy-App

Its another app
GNU General Public License v3.0
0 stars 0 forks source link

Simpler custom settings #24

Closed cailadd90 closed 1 year ago

cailadd90 commented 1 year ago

I've tested the effect of using less data in classifying partially inundated cases to speed up the analysis, and there's no issue. Benchmarking showed that runtimes plateaued at 25% of the data being used. Any less shows no improvement in performance.

'fun_hydrology.R' The default value of the 'chop' argument is now 0.25 and I have removed the warning message.

'fun_iu_hydrology.R' I have removed the slider option in the custom settings.

mcwimm commented 1 year ago

Great! And how does the value influence the outcome itself?

cailadd90 commented 1 year ago

Great! And how does the value influence the outcome itself?

Using the default target dataset as an example, and using different proportions of the data for detecting partially inundated cases (100, 75, 50, 25, 10% of the data), the largest difference in fully inundated cases being classified as partially inundated was 322 minutes over the length of the survey - with no clear trend depending on the proportion of data used. Using the full dataset (100%) as a baseline, the largest difference was 150 minutes. The length of the survey is 35,376 minutes, so the range represents 0.9% of the data. Given the small variation in output, and the reduced computation rates, I'm happy to use 25% of the data in partial inundation detection as a fixed amount. This can be explained in the manuscript, plus an additional appendix with these attached plots.

PartialInundationClassification

Benchmarking

mcwimm commented 1 year ago

Ok, cool! Then we keep it like this. However, the app crashes at some point. Need some time to fix this.

mcwimm commented 1 year ago

Ok. I think I found the error: 'chop' needed to be removed in several places, e.g. for reference and in the parameter collection from the frontend...

It would be great if you can test this. For me an error occured when using Target/Reference : Custom settings : Apply custom settings If it works for you, this PR is ready to be merged.

cailadd90 commented 1 year ago

Ah great to have found the error so quickly - I can pull the latest version by running 'git fetch --all' then 'git checkout origin/master' in the terminal, correct?

I'm like the Texas chainsaw man hacking away at App functions without knowing exactly what they do! I would actually really value seeing what you fixed so I can learn from it. Maybe you could show me how to check changes in GitHub sometime.

mcwimm commented 1 year ago

Ah great to have found the error so quickly - I can pull the latest version by running 'git fetch --all' then 'git checkout origin/master' in the terminal, correct?

git fetch --all git checkout origin/SimplerCustomSettings

mcwimm commented 1 year ago

I'm like the Texas chainsaw man hacking away at App functions without knowing exactly what they do! I would actually really value seeing what you fixed so I can learn from it. Maybe you could show me how to check changes in GitHub sometime.

:laughing: You can look at changes here on Github by navigating to 'Files changed' image

Your start was correct. The only problem was that the 'chop' parameter occured in several other places as well.

cailadd90 commented 1 year ago

Ok. I think I found the error: 'chop' needed to be removed in several places, e.g. for reference and in the parameter collection from the frontend...

It would be great if you can test this. For me an error occured when using Target/Reference : Custom settings : Apply custom settings If it works for you, this PR is ready to be merged.

Yes it works for me! Changed custom settings a few times and the code ran each time.

Without thinking, I did try a minimum tilt value of 95, and it - unsurprisingly - return an error. I wonder if there's a way to limit the possible inputs to a range of 0 - 90? I'll add this to issues.

mcwimm commented 1 year ago

Without thinking, I did try a minimum tilt value of 95, and it - unsurprisingly - return an error. I wonder if there's a way to limit the possible inputs to a range of 0 - 90? I'll add this to issues.

True. This seems to be a shiny issue. Normally you can define min/max but it's not recognized. We'll need to find a manual solution.