randlab / ArchPy

ArchPy - Stochastic geological modeling
https://archpy.readthedocs.io/en/latest/
GNU General Public License v3.0
45 stars 5 forks source link

Adding gaps #8

Closed alecomunian closed 1 year ago

alecomunian commented 1 year ago

Hi Ludovic, I have some questions that should help me to better understand the function add_gap in inputs.py. If I understand correctly, when there is some missing information in your data, then add_gap tries to fill the gap by adding a "None" unit/facies in that segment of borehole. Hereinafter my questions: 1) Maybe in the previous merge I asked you, I was too strict by suggesting an "Error" when the top of a facies is below the top of a borehole. Therefore, if you want I have just committed a new version with a "warning" in place of an "error message". If you want I can ask you another "merge" for this. 2) When I edit inputs.py in Spyder, Pyflakes warns about the fact that the variable bot is undefined (on the line if i > 0 and bot != top: and the one just below). There might be the risk that when you call add_gap with this variable bot undefined this could create some trouble? 3) In some cases with my data I have SettingWithCopyWarning: A value is trying to be set on a copy of a slice from a DataFrame. This complaints often comes from add_gap, at the line lay=idata.iloc[0]. If I replace this with something like lay=copy.deepcopy(idata.iloc[0]) the warning disappears. Do you think doing this edit might make sense? 4) To further test add_gap I intentionally raised the $z$ coordinate of one well above the first top definition. What should be the behavior of the code in this case?

Thank you in advance. Bests, Alessandro

LudovicSchorpp commented 1 year ago

Hello Alessandro,

Thank you for your questions/remarks. I have considered and integrate them to the next commit but to answer you in details:

  1. Yes maybe I think it is better to warn the user but not to block the code completely. I have updated the code in order to give more informations about these issues (which borehole, which altitude).
  2. Yes it is not the best but no issue is possible for this case as this add gap is only activated when i>0 and if it is the case, then bot will be defined. So no problem here normally.
  3. Yes it makes sense but I'm wondering if it could slow the code. Maybe I should think to another way to do it but for now I will simply make a copy.
  4. This was a case that I didn't consider before but I have implemented it now. So it will consider that the information given are correct and will assume that there is a gap between the top of the borehole and the first unit/facies layer occurence. A "None layer" will then be added.

I hope it answers to your questions and thank you again to improve ArchPy !

Amitiés, Ludovic