Update PGS to contain tree based lithotype rules#387
Update PGS to contain tree based lithotype rules#387EJRicketts wants to merge 14 commits intoGeoStat-Framework:mainfrom
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. |
LSchueler
left a comment
LSchueler
left a comment
There was a problem hiding this comment.
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.
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.
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.
There was a problem hiding this comment.
No problem. I will leave this for another time :)
| 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. |
| yielding a fully *pluri*gaussian simulation. | ||
| This may sound more complicated than it is; we will clarify everything | ||
| in the examples that follow. | ||
|
|
|
All comments have been addressed 😊 The main changes related to adding a full unit test and making sure docstrings would link in the documentation |
2 participants
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 addedDecisionTreeclass being a child ofPGS? I thought to add as a separate class inpgs.py, but having it as a child seemed cleaner