Skip to content

Update PGS to contain tree based lithotype rules #387

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

EJRicketts
Copy link

When using spatial lithotypes, we often fall into the trap of restricting ourself to using the same number of input spatial random fields as the dimensions of the simulation domain. PGS as a method does not require this, so it would be nice to be able to use an arbitrary number of random fields. This is what the decision tree type implementation allows for.

Much of the PGS class has been refactored, and some new examples have been added to highlight how to use the tree based lithotype architectures.

Questions:

  • Would it be worth adding the ellipse() and ellipsoid() functions as helper functions, or just allow people to define their own? Other shapes could also be added
  • Are we happy with the DecisionTree class being a child of PGS? I thought to add as a separate class in pgs.py, but having it as a child seemed cleaner

@LSchueler
Copy link
Member

Hi Evan,

wow, that was fast! Thank you so much for this PR and for working on enhancing the PGS capabilities of GSTools.
I just activated the CI checks for you PR.
You should run black, isort, and pylint on your code. You can see the exact commands we use in this file on lines 38, 46, and 50 (you should drop the --check, --preview, ... flags).

I should have time next week for a code review. I'm looking forward to it!

@LSchueler LSchueler assigned LSchueler and unassigned LSchueler Jun 27, 2025
@EJRicketts
Copy link
Author

This is the beauty of having a bit more free time!

I have gone through and checked through with black, isort, and pylint and things look all good! pylint does throw some warnings for Module gstools.covmodel.tools and Module gstools.transform.array, but I have left these

I also need to see why test_assertions() is not working in TestPGS, but I'm not sure why as it is using the original PGS implementation

@LSchueler
Copy link
Member

Hi Evan,

I finally found some time to have a look at your code, sorry for having to wait for so long. Your examples are beautiful!!

Here are a few first remarks.

Copy link
Member

@LSchueler LSchueler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far this looks really good. There are two things which are missing so far.

  1. Unit tests are still missing for the new features.

  2. The parameters in the docstrings should generate links to their type definitions, e.g. with

:any:`DecisionNode`

You can have a look at the docstrings in e.g. src/gstools/field/srf.py.

But it's really nice that you mention the possible exceptions. One day, we should add that to all docstrings...

I'll need some more time to go through your code in detail, but I really hope to find some more time this week.

more flexible approach can be taken such that the lithotype is represented as a
decision tree. More specifically, this is given as a binary tree, where each
node is a decision based on the values of the spatial random
fields [Sektnan et al., 2024](https://doi.org/10.1007/s11004-024-10162-5). The
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this doesn't work in sphinx. I just realised that I made the same mistake in my own PGS examples. I'll have to fix those. The correct syntax is

`Sektnan et al., 2024 <https://doi.org/10.1007/s11004-024-10162-5>`_

Comment on lines +44 to +52
def ellipse(data, key1, key2, c1, c2, s1, s2, angle=0):
x, y = data[key1] - c1, data[key2] - c2

if angle:
theta = np.deg2rad(angle)
c, s = np.cos(theta), np.sin(theta)
x, y = x * c + y * s, -x * s + y * c

return (x / s1) ** 2 + (y / s2) ** 2 <= 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be a great idea to put functions like ellipse and ellipsoid directly into GSTools' code, probably into src/gstools/field/tools.py.
But I could also do that some time after this PR is merged.

requirement of the PGS algorithm. In fact, it is possible to use multiple
spatial random fields in PGS, which can be useful for more complex lithotype
definitions. In this example, we will demonstrate how to use multiple SRFs in
PGS. In GSTools, this is done by utlising the tree based architecture.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spelling

yielding a fully *pluri*gaussian simulation.
This may sound more complicated than it is; we will clarify everything
in the examples that follow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this text :-)

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