arbrandt / OPGEE

Oil Production Greenhouse Gas Emissions Estimator
18 stars 5 forks source link

Calculation issues with some smart defaults #221

Closed qlangfitt closed 4 years ago

qlangfitt commented 4 years ago

I was noticing that for a particular field I would get two possible sets of results depending on which field I had run before it. I tracked it down to an issue with the way the smart default is calculated for water reinjection. If water reinjection is left blank, it will fill in with a 0 if water flooding is on or a 1 if water flooding is off. This can be a problem when the user has also not entered a water flooding input because in that case the water reinjection smart default will be based on whether the previously run field was using water flooding or not.

I think this can be traced back to the VBA code that skips blank cells at line 90. I understand why blank cells are skipped (so there are no blanks on the Active Field sheet), but I think there also needs to be a step prior to that that sets all of the Active Field inputs to some consistent default value before the smart defaults start doing their thing. I'm not exactly sure how to best do this, but perhaps the "mean" column or a new column that holds those defaults?

This seems like it might be an issue more broadly, but I haven't run into it otherwise so I might be missing something like this that already happens? If so, there is still a problem with water reinjection, maybe because it and water flooding depend on each other so there is some kind of ordering issue.

smasnadi commented 4 years ago

I think the problem goes back to this line of VBA:

      'Alternative method that copies only non-blank cells from the inputs sheet and avoids looping over all rows
      Worksheets("Inputs").Range("G9:G115").Offset(0, i).Copy
      ActiveField.Range("J48:J154").PasteSpecial SkipBlanks:=True

If we copy and paste ALL Inputs cells to the Active Field cells, the issue will be resolved. Correct?

qlangfitt commented 4 years ago

I assume the SkipBlanks statement is there because it's not ideal to copy blank cells to the active field sheet. I don't think anything will break, strictly speaking, if we copy the blank cells, but many of the smart defaults will simply end up defaulting to the _value_iffalse portion of their IF statements when the logical they draw on is blank. Unless those are designed to hold the true default in _value_iffalse, I worry that this could lead to inputs that are not the desired defaults (though they would be consistent).

It's also possible that blank cells will break the code when more advanced smart defaults are set like the productivity index, but I just don't know.

qlangfitt commented 4 years ago

I found another instance of this happening with a different smart default. If oil production volume and GOR are both not entered, the default oil production volume will be based on the GOR of the previous field run (checking if it's a gas or oil field) not on the smart default GOR of the current field.

arbrandt commented 4 years ago

I am concerned that the "mean" column might refer to some of the active cells. I think we need something totally static that will always set the column to the exact same values regardless of the last run. The best option I think is the unchanging and totally filled row for the default field from the inputs sheet. So the workflow will be to copy the default column (column 0 in the VBA numbering scheme) over the (old, residual) active field row. Then go to the column you are actually wanting to copy and only move the non-blanks over to active field. Will implement and test now.

arbrandt commented 4 years ago

Added the following code:

   'Reset the active row to the default settings so that information does not incorrectly propogate from one run to another
    Worksheets("Inputs").Range("G9:G115").Copy
    ActiveField.Range("J48:J154").PasteSpecial SkipBlanks:=True