Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate changes made for the ML near-well model #4

Merged
merged 1 commit into from
Aug 2, 2024
Merged

Conversation

pschultzendorff
Copy link
Collaborator

@pschultzendorff pschultzendorff commented Jun 19, 2024

Finally found time to fix everything.

Proposed changes

  • Introduces an upscaling module which features a BaseUpscaler abstract base class, which provides methods to upscale data from fine-scale radial ensemble simulations to a coarse-scale Cartesian grid.
  • Adds an analysis module to analyze sensitivity of NNs w.r.t. their inputs.
  • Adds functionality to enable determinism for ensemble runs and NN training and ensures this is correctly integrated in all relevant modules.
  • Adds functionality to extract only OPM flags from an pyopmnearwell deck. I needed this for some ensemble related stuff or something.
  • Adds a new formula that calculates only the rock part of Peaceman's well index.
  • Fixes bugs in scaler_layers and nn.scale_and_evaluate
  • Lots of maintenance.
  • Restructures the tests of different models and geometries. They kept failing locally for me, hence I restructured them. Now, they use a unified setup, work independent of the order they are run in, and all simulations files are created in tmp directories instead of the test directory. Note that test_plot_comparison runs only on one simulation result and not on multiple, I could change this though.
  • Removes some functionality that I introduced at some point, but never used, e.g., the "empty_well" option in the deck.
  • Removes the standardwell_impl and standardwell makos. They are kind of obsolete (I wrote new ones for the near-well article) and also very specific to a usecase, so I do not think they fit too well inside pyopmnearwell. In the integration module I link to the article repo for examplary usage.

And probably some more small changes that I forgot about in this list.

Practicalities

  • I left some TODO flags in the code. They mark functionality not really needed at this point, but maybe at some point I will continue working on them.

Types of changes

What types of changes does this PR introduce to pyopmnearwell?

  • Minor change (e.g., dependency bumps, broken links).
  • Bugfix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • Testing (contribution related to testing of existing or new functionality).
  • Documentation (contribution related to adding, improving, or fixing documentation).
  • Maintenance (e.g., improve logic and performance, remove obsolete code).
  • Other:

Checklist

  • The documentation is up-to-date.
  • Static typing is included in the update.
  • This PR does not duplicate existing functionality.
  • The update is covered by the test suite (including tests added in the PR).
  • If new skipped tests have been introduced in this PR, pytest was run with the --run-skipped flag.

@pschultzendorff
Copy link
Collaborator Author

When this is done, we can also remove the dev_2 and development branches to clean up a little.

@pschultzendorff
Copy link
Collaborator Author

I am not quite sure why pylint fails, as it gave me a 10.0 score locally. It's probably a different pylint version or something. I will fix everything and try to reinstall pylint locally.

@pschultzendorff
Copy link
Collaborator Author

I updated pylint, mypy, black, and pytest to the newest versions locally and everything runs fine. The pylint issues on github are due to pylint not recognizining the static typing correctly. Both locally and on github, pylint version 3.2.3 is run, so I have no idea why it does not work.

@daavid00
Copy link
Member

Hey, thanks @pschultzendorff for the nice work, just a few things:

  • Could you update the Python version to 3.10 in the CI.yml? This should fix the pylint issue (since Ubuntu 24.04 has been released, then now we could move to this higher version of Python instead of 3.8). If this works, then we should remove the badges Python3.8 and Python3.9 from the README.md and update the setup.py (L 41).
  • Could you update the scripts using ecl to resdata (ecl has been deprecated)? Then, we can remove ecl from the requirements.txt.
  • Before merging, please squash all commits into one.

@pschultzendorff
Copy link
Collaborator Author

I swapped from ecl to resdata in the modules ml.ensemble and ml.ecl_dataset (now called ml.resdata_dataset). However, I won't fix all the examples as that is too much work.

@pschultzendorff
Copy link
Collaborator Author

I took the chance to change test_plot_comparison to run on multiple results. Flow aborts the co2_eor simulation due to some reason, so I removed it from test_plot_comparison. test_co2_eor is still successful, because the UNRST file is created (but no summary file)

@daavid00
Copy link
Member

daavid00 commented Aug 2, 2024

It looks very good, thanks for the nice coding, merging :).

@daavid00 daavid00 merged commit b434fe3 into main Aug 2, 2024
1 check passed
@pschultzendorff pschultzendorff deleted the dev_2 branch August 20, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants