Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: 235 layers not calculated correctly waterside #240

Merged
merged 17 commits into from
Dec 18, 2024
Merged

fix: 235 layers not calculated correctly waterside #240

merged 17 commits into from
Dec 18, 2024

Conversation

Carsopre
Copy link
Collaborator

@Carsopre Carsopre commented Dec 17, 2024 •
edited
Loading

Issue addressed

Solves #235

Code of conduct

  • I HAVE NOT added sensitive or compromised (test) data to the repository.
  • I HAVE NOT added vulnerabilities to the repository.
  • I HAVE discussed my solution with (other) members of the KOSWAT team.

What has been done?

  • When a new geometry upper points (representing a "coating layer surface") intersects its previous geometry then we calculate the added geometry based on the intersection with the very first point that discerns from its base relative.

Checklist

  • Tests are either added or updated.
  • Branch is up to date with master.
  • Updated documentation if needed.

Additional Notes (optional)

...

@Carsopre Carsopre changed the base branch from master to feat/181-extend-calculations-for-having-berm-as-part-of-input-profile December 17, 2024 16:23
@Carsopre Carsopre changed the base branch from feat/181-extend-calculations-for-having-berm-as-part-of-input-profile to master December 17, 2024 18:49
@Carsopre Carsopre changed the base branch from master to feat/181-extend-calculations-for-having-berm-as-part-of-input-profile December 17, 2024 18:49
@Carsopre Carsopre marked this pull request as ready for review December 17, 2024 19:13
@Carsopre Carsopre force-pushed the fix/235-layers-not-calculated-correctly-waterside branch from ebdb951 to 7a0f9e3 Compare December 18, 2024 09:34
@Carsopre Carsopre changed the base branch from feat/181-extend-calculations-for-having-berm-as-part-of-input-profile to master December 18, 2024 09:35
@Carsopre
test: Updated scenario test cases
2adc7e3
@Carsopre
Squashed commit of the following:
177edaa 
commit d8529ac
Author: Ardt Klapwijk <[email protected]>
Date:   Mon Dec 16 15:38:54 2024 +0100

    test: update reference data

commit da17886
Merge: 5710c6f 2d92a4c
Author: Ardt Klapwijk <[email protected]>
Date:   Mon Dec 16 15:34:50 2024 +0100

    Merge branch 'master' into feat/181-extend-calculations-for-having-berm-as-part-of-input-profile

commit 5710c6f
Author: Peter de Grave <[email protected]>
Date:   Mon Dec 16 11:08:39 2024 +0100

    #181 test cases

    Added test cases to reproduce bugs #233 and #235

commit 5083940
Author: peterdgr <[email protected]>
Date:   Mon Dec 9 15:35:27 2024 +0100

    feat: 181 extend calculations for having berm as part of input profile

    Enhancements for stability wall input profile

commit 044f4d9
Author: peterdgr <[email protected]>
Date:   Mon Dec 9 15:34:56 2024 +0100

    feat: 181 extend calculations for having berm as part of input profile

    Use unbuild ground price for VPS

commit 78e5459
Author: peterdgr <[email protected]>
Date:   Mon Dec 9 15:34:23 2024 +0100

    feat: 181 extend calculations for having berm as part of input profile

    Include dikesection names in export

commit 850914a
Author: peterdgr <[email protected]>
Date:   Mon Dec 9 15:33:19 2024 +0100

    feat: 181 extend calculations for having berm as part of input profile

    Prevent error at export of shapefile

commit f393ff4
Merge: e60ed60 3ab9c75
Author: Ardt Klapwijk <[email protected]>
Date:   Wed Nov 13 11:36:15 2024 +0100

    Merge branch 'master' into feat/181-extend-calculations-for-having-berm-as-part-of-input-profile

commit e60ed60
Merge: 356764e 5450a2c
Author: Ardt Klapwijk <[email protected]>
Date:   Wed Nov 13 10:56:49 2024 +0100

    Merge branch 'master' into feat/181-extend-calculations-for-having-berm-as-part-of-input-profile

commit 356764e
Merge: fbbe933 d942e77
Author: Carles S. Soriano Pérez <[email protected]>
Date:   Mon Nov 11 11:44:57 2024 +0100

    Merge branch 'master' into feat/181-extend-calculations-for-having-berm-as-part-of-input-profile

commit fbbe933
Merge: 93e31c3 b05ad5e
Author: Carles S. Soriano Pérez <[email protected]>
Date:   Mon Nov 11 09:22:17 2024 +0100

    Merge branch 'master' into feat/181-extend-calculations-for-having-berm-as-part-of-input-profile

commit 93e31c3
Author: Carles S. Soriano Pérez <[email protected]>
Date:   Fri Nov 8 12:48:03 2024 +0100

    fix: 220 last row error in strategy buffering and clustering

commit 2469306
Author: Ardt Klapwijk <[email protected]>
Date:   Wed Nov 6 15:06:22 2024 +0100

    chore: fix cofferdam bug

commit 68991a7
Author: Ardt Klapwijk <[email protected]>
Date:   Wed Nov 6 14:57:28 2024 +0100

    chore: fix vps bug

commit c367b7e
Author: Ardt Klapwijk <[email protected]>
Date:   Wed Nov 6 14:49:02 2024 +0100

    chore: fix bugs

commit 36c3e42
Merge: 56816c8 b672c7b
Author: Ardt Klapwijk <[email protected]>
Date:   Wed Nov 6 12:25:29 2024 +0100

    Merge branch 'master' into feat/181-extend-calculations-for-having-berm-as-part-of-input-profile

commit 56816c8
Author: peterdgr <[email protected]>
Date:   Wed Nov 6 11:11:28 2024 +0100

    feat: 181 extend calculations for having berm as part of input profile

    Update scenario cases and base profiles

commit 16b8dda
Author: peterdgr <[email protected]>
Date:   Fri Nov 1 09:49:47 2024 +0100

    feat: 181 extend calculations for having berm as part of input profile

    Extend calculations for having berm as part of input profile. Added stability Wall and CofferDam.

commit accfc5b
Author: peterdgr <[email protected]>
Date:   Wed Oct 30 17:53:24 2024 +0100

    feat: 181 extend calculations for having berm as part of input profile

    Improve input profile calculations to accommodate dikes with existing berms at polderside
@Carsopre
chore: Removed unnecessary file
cd30983
@Carsopre
chore: Simplified (and improved) how to determine the added geometry
bf6e5e9
@Carsopre
test: Added new reference data
c8b152e
@Carsopre Carsopre changed the title Fix/235 layers not calculated correctly waterside fix: 235 layers not calculated correctly waterside Dec 18, 2024
@Carsopre Carsopre linked an issue Dec 18, 2024 that may be closed by this pull request
Layers not calculated correctly (waterside) #235
Open
2 tasks
@Carsopre Carsopre changed the base branch from master to feat/181-extend-calculations-for-having-berm-as-part-of-input-profile December 18, 2024 14:03
@Carsopre Carsopre mentioned this pull request Dec 18, 2024
Open
2 tasks
@Carsopre
Merge branch 'feat/181-extend-calculations-for-having-berm-as-part-of…
47bb831 
…-input-profile' into fix/235-layers-not-calculated-correctly-waterside
ArdtK
ArdtK requested changes Dec 18, 2024
View reviewed changes
Copy link
Contributor

@ArdtK ArdtK left a comment

Choose a reason for hiding this comment

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

Few comments.
In addition some general remarks:

  • Would you add a legend to explain the colored dots and lines on the profile plots?
  • A README on subpackage reinforcement_layers would be useful, with an explanation on what e.g. a core, layer, relative geometry, etc are.

...at/dike_reinforcements/reinforcement_layers/standard_reinforcement_layers_wrapper_builder.py
Copy link
Contributor

Choose a reason for hiding this comment

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

Method _get_coating_layers became quite large. Can it be split in smaller methods?
Some more comments would be helpful too, as there's quite a lot going on.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Although my time is up I'll do it.

...at/dike_reinforcements/reinforcement_layers/standard_reinforcement_layers_wrapper_builder.py Outdated Show resolved Hide resolved
ArdtK
ArdtK reviewed Dec 18, 2024
View reviewed changes
.gitignore Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this folder exist?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, in fact there's a test scrapping data from it. But we do not want to commit these tests as they can be potentially large and/or have sensible data.

@Carsopre
Copy link
Collaborator Author

Few comments. In addition some general remarks:

* Would you add a legend to explain the colored dots and lines on the profile plots?

* A README on subpackage `reinforcement_layers` would be useful, with an explanation on what e.g. a core, layer, relative geometry, etc are.

Unfortunately I do not have time for the legend nor the readme, these will be addressed in a separate issue as already discussed earlier this week: #239

@Carsopre Carsopre merged commit cfd2a64 into feat/181-extend-calculations-for-having-berm-as-part-of-input-profile Dec 18, 2024
0 of 2 checks passed
@Carsopre Carsopre deleted the fix/235-layers-not-calculated-correctly-waterside branch December 18, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ArdtK ArdtK ArdtK requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Layers not calculated correctly (waterside)
2 participants
@Carsopre @ArdtK