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.

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

Repeatedly creating 1D2D links results in error due to double entries #546

Open
Kreef opened this issue May 16, 2023 · 1 comment
Open
Labels
priority: medium type: bug Something isn't working

Comments

@Kreef
Copy link

Kreef commented May 16, 2023

Describe the bug
When adding 1D2D links more than one time, for example some embedded and some later links, issues arise due to double counting of link ids. I found the issue and propose a solution below.

To Reproduce
within the Link1d2d class in HYDROLIB-core/hydrolib/core/dflowfm/net/models.py, the process function appends new links to exising links. The code below is currently in HYDROLIB:
def _process(self) -> None:
"""
Get links from meshkernel and add to the array with link administration
"""
contacts = self.meshkernel.contacts_get()

    self.link1d2d = np.append(
        self.link1d2d,
        np.stack([contacts.mesh1d_indices, contacts.mesh2d_indices], axis=1),
        axis=0,
    )
    self.link1d2d_contact_type = np.append(
        self.link1d2d_contact_type, np.full(contacts.mesh1d_indices.size, 3)
    )
    self.link1d2d_id = np.append(
        self.link1d2d_id,
        np.array([f"{n1d:d}_{f2d:d}" for n1d, f2d in self.link1d2d]),
    )
    self.link1d2d_long_name = np.append(
        self.link1d2d_long_name,
        np.array([f"{n1d:d}_{f2d:d}" for n1d, f2d in self.link1d2d]),
    )

Here, self.link1d2d and self.link1d2d_contact_type are handled correctly. However, self.link1d2d_id and self.link1d2d_long_name take the current entries and apppend the names of all 1d2d link in self.1d2d link. As self.1d2dlink contains both the old and new entries (due to the append earlier), self.link1d2d_id will end up with double entries for all entries that were already present. This leads to errors when writing the DIMR configuration.

A solution would be to change the process function into the following, solving the issue:

def _process(self) -> None:
    """
    Get links from meshkernel and add to the array with link administration
    """
    contacts = self.meshkernel.contacts_get()

    self.link1d2d = np.append(
        self.link1d2d,
        np.stack([contacts.mesh1d_indices, contacts.mesh2d_indices], axis=1),
        axis=0,
    )
    self.link1d2d_contact_type = np.append(
        self.link1d2d_contact_type, np.full(contacts.mesh1d_indices.size, 3)
    )

    self.link1d2d_id = np.array([f"{n1d:d}_{f2d:d}" for n1d, f2d in self.link1d2d])

    self.link1d2d_long_name = np.array([f"{n1d:d}_{f2d:d}" for n1d, f2d in self.link1d2d])

Expected behavior
No double counting and correctly saving the model

@veenstrajelmer
Copy link
Collaborator

veenstrajelmer commented Jan 15, 2024

Hi @Kreef, thanks for creating this issue. For the Mesh2d class these type of issues were fixed by aligning directly with the underlying meshkernel class at the end of last year, but we did not get around implementing this for the other hydrolib-core classes. I will link the issue where we plan to do this for future reference: #576

@priscavdsluis priscavdsluis added type: bug Something isn't working priority: medium labels Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: medium type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants