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

Add Recoordination Process with Ghost Point #12

Merged
merged 105 commits into from
Apr 3, 2024
Merged

Add Recoordination Process with Ghost Point #12

merged 105 commits into from
Apr 3, 2024

Conversation

leorudczenko
Copy link
Contributor

@leorudczenko leorudczenko commented Feb 13, 2024

Summary

This merge request implements the following:

Description

Recoordination

The recoordination functionality has been implemented according to Matt's specifications and feedback, as seen in #4. For this functionality, the environment has been modified to include numpy to perform the affine transformation.

Ghost Point

A ghost point feature has been added following feedback seen in #6. This feature can be enabled/disabled by the user, as per feedback from Matt.

Code Refinement

To accommodate both of the new features, much of the internal logic has been overwritten to make it more versatile and less complex. For example, many of the parsing methods have been moved into a separate file, away form the front-end code. These logic changes have also solved the bug detailed in #9.

Moreover, due to the addition of numpy, the compiled exe file was considerably larger, initially being 270MB. Previously, the exe file was only 37MB, and so some additional changes have been made to the environment to reduce this size down. As it stands, the new exe file size is 75MB with all the new functionality and numpy included. The solution used comes from 2 discussions online:

Many tests have been added to ensure all of the new functionality works as expected.

Documentation

  • The instructions have been updated to include the recoordination process and the ghost point option.
  • The readme has been updated to include the full mermaid diagram using GitHub's new integration. This was previously not possible as the diagram was too large.
  • The list of tests has been removed from the readme as it was unnecessary.

Closes #4, #6, #9

@leorudczenko leorudczenko linked an issue Feb 13, 2024 that may be closed by this pull request
@leorudczenko leorudczenko self-assigned this Feb 13, 2024
@leorudczenko leorudczenko marked this pull request as ready for review February 13, 2024 14:23
@leorudczenko
Copy link
Contributor Author

The majority of these changes had already been reviewed by @volcan01010, but following the discussion with Matt some additional changes were made which can be seen in this commit: 66b4373

@ximenesuk has reviewed these additional changes to ensure the modified transformation logic makes sense.

@leorudczenko leorudczenko merged commit 9794bca into main Apr 3, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants