equinor / flownet

FlowNet - Data-Driven Reservoir Predictions
GNU General Public License v3.0
63 stars 29 forks source link

Issue #235 mitchell 2D #246

Closed LonnekevB closed 3 years ago

LonnekevB commented 3 years ago

Insert a description of your pull request (PR) here, and check off the boxes below when they are done.


Contributor checklist

LonnekevB commented 3 years ago

@wouterjdb Status update:

wouterjdb commented 3 years ago

Woops, didn't want to close. Tried to cancel my message only...

  • Tests implemented for 2D and 3D Mitchells best candidate.

☑️

  • Adjustments made to the _mitchells.py code to make it work also for 2D. This works, but the 2D issue is still popping up further on in the code (_network_model.py > def _create_connection_groups) where / (maxz - minz) is a division by 0. Haven't had the time to look into this part of the code yet, so haven't fixed this yet.

This will be part of follow-up work. You could write an issue on this. No need to include it in this PR.

  • Some issues with changing print to logging. Do we want to do this in the whole code? Than I suggest to make that a separate issue and do it in a neat way for the whole code. And do you want to log it to a file or still just to the terminal?

Good point. Yes, make it into an issue. Should we not change to logging at all then in this PR?

  • Having some issues with the CI tests in github. It currently fails on style and linting, but as I don't get this error locally I am having some trouble to figure out what the issue is.

If you install a fresh virtual environment on an older version of Ubuntu you might also have an old version of pip. I think that could lead to not getting the latest version of something like black/pylint (bit of guessing here). Try to update pip:

pip install --upgrade pip

And update black, pylint and mypy afterwords.

pip install --upgrade bacl pylint mypy

Then run the tests:

black src tests
pylint src/flownet
mypy --ignore-missing-imports src

The error by the way at the moment is

src/flownet/network_model/_mitchell.py:3:0: W0611: Unused import logging (unused-import)

You are importing the logging package but not using it.

LonnekevB commented 3 years ago

@wouterjdb : The Brugge CI test runs fine locally. But I am currently running it in python 3.6.9. The CI test that fails is the python 3.7 one. I will try running it in python 3.7 tomorrow.