-
Notifications
You must be signed in to change notification settings - Fork 75
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
base: main
Are you sure you want to change the base?
Conversation
…ple scripts for 2D and 3D simulations
…hotype calculation function
Hi Evan, wow, that was fast! Thank you so much for this PR and for working on enhancing the PGS capabilities of GSTools. I should have time next week for a code review. I'm looking forward to it! |
This is the beauty of having a bit more free time! I have gone through and checked through with I also need to see why |
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. |
There was a problem hiding this 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.
-
Unit tests are still missing for the new features.
-
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 |
There was a problem hiding this comment.
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>`_
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 |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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. | ||
|
There was a problem hiding this comment.
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 :-)
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:
ellipse()
andellipsoid()
functions as helper functions, or just allow people to define their own? Other shapes could also be addedDecisionTree
class being a child ofPGS
? I thought to add as a separate class inpgs.py
, but having it as a child seemed cleaner