dams-mcda / Dams-MCDA

Emma Fox R/Shiny Project with a docker server configuration
1 stars 0 forks source link

attempt at normalizing all dams by adding one for loop #105

Closed sythel closed 5 years ago

sythel commented 5 years ago
sythel commented 5 years ago

@elbfox does this seem to simplify the normalization process?

elbfox commented 5 years ago

Yes, the changes seem to streamline the normalization for individual dams, but I'd like to be able to look through it more closely to make sure. I just skimmed the changes because I'm in the middle of throwing together a quick Excel version of the model to verify that it's calculating properly. I'll hopefully get to checking this over before the end of the day. I'm going to need to test to see if it calculates correctly, a concern I have is whether we can still break out the weighted values (prefs*normalized data) to graph individual dam criteria/alternatives in this version.

elbfox commented 5 years ago

Forgive my ignorance about version control, but is this something that we could either: 1) pull into the WSM.R script (this one is now functionally incorrect) to keep the file "WSM_graphs_test.R" consistent with what I have in my version (where I do have the normalization process just repeated for each individual dam, or 2) rename something else, taking the useful/appropriate pieces from the WSM.R script to make this into the WSM function to be called by server.R

My reason for asking (and let me know if there is a better solution, here) is because I want to maintain my changes to the file while running your version and testing against my version and the dataset I created, with each step calculated: MCDA_Check_CalcsGraphs.xlsx

sythel commented 5 years ago

sounds like you want to merge the two branches but not commit/push the changes. you could make another branch as experimental and pull request or merge GraphTesting_Normalization into it

assumming you're on the branch with your latest changes git checkout -b GraphTesting_experimental git merge GraphTesting_Normalization

once you push (if you choose to) then it will appear on github

elbfox commented 5 years ago

Thanks. I made a GraphTesting_DataUpdate branch, named because in addition to the individual dam normalization/WSM calc (PrefsMatrix*NormalizedMatrix) for each dam (with graphs for testing) it has changes to the DamsData.csv and then the DamsData array step (where alternatives get defined and then bound as an array). I requested a merge into GraphTesting because that was my last pull and there were no conflicts. Once you review that, let me know and we can merge that pull request first before merging this one (unless you don't think that makes sense).

elbfox commented 5 years ago

Merging my GraphTesting_DataUpdate branch generated conflicts, but it means that my version of WSM_graphs_test.R is archived and I can pull from the GraphTesting_Normalization branch and resolve these conflicts locally, then check with my dataset before approving. I'm going to do this over the weekend so I can have a few solid hours to go through it carefully.

sythel commented 5 years ago

i did a quick merge, may need to revert if I messed up.

Here is reference to code before the merge if you need to reference: https://github.com/dams-mcda/Dams-MCDA/tree/b98ea514677c4ea05f705a0707417005d59b7d7b

elbfox commented 5 years ago

Individual dam normalization checks out. Did make some changes to individual dam summed score calc (it was summing within matrix_levs_ind and not matrix__cols) within new branch "GraphChecking" to compare individual bar graphs and stacked bar graph results. Also reworking multi-dam stuff to call Sam's original f_nrge dataset, which is pre-normalized.