-
Notifications
You must be signed in to change notification settings - Fork 39
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
re-enabling 2D for chunk. not clear why this option was removed. #554
re-enabling 2D for chunk. not clear why this option was removed. #554
Conversation
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.
Menno will have to decide on this PR (I think you discussed it with him), but I would suggest to delete the line completely if you are not sure why it was introduced in the first place. Leaving commented code is a recipe for slowly cluttering the whole code with untrustworthy and outdated information.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #554 +/- ##
==========================================
+ Coverage 93.49% 93.54% +0.05%
==========================================
Files 92 92
Lines 6346 6340 -6
==========================================
- Hits 5933 5931 -2
+ Misses 413 409 -4
Continue to review full report in Codecov by Sentry.
|
|
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.
Yes, I agree, please delete the line. Otherwise good to go. If we find an issue in the future, we can document directly what the issue is and/or directly try to fix it.
cad0ce1
into
GeodynamicWorldBuilder:main
This option existed before May 2023. It is not clear why it was removed and the assert added. However, Magali and Becky both use this option in the previous version (0.5 to check input structure before running in Aspect), without any problems and can't change to the new version without it. So, since we don't currently know what the problem is, I have removed the assert for now to allow the 2D option to be used with the chunk option and the grid input file.