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

Fix issue 353 #417

Merged
merged 5 commits into from
Mar 25, 2023
Merged

Fix issue 353 #417

merged 5 commits into from
Mar 25, 2023

Conversation

sandro-elsweijer
Copy link
Collaborator

@sandro-elsweijer sandro-elsweijer commented Jan 26, 2023

Describe your changes here:
Closes #353
Based on PR #380

All these boxes must be checked by the reviewers before merging the pull request:

As a reviewer please read through all the code lines and make sure that the code is fully understood, bug free, well-documented and well-structured.

  • The author added a BSD statement to doc/ (or already has one)

  • The code compiles without warning in debugging and release mode, with and without MPI (this should be executed automatically in a github action)

    If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):

    • Should this use case be added to the github action?
    • If not, does the specific use case compile and all tests pass (check manually)
  • All tests pass (in various configurations, this should be executed automatically in a github action)

  • New source/header files are properly added to the Makefiles

  • New Datatypes are added to t8indent_custom_datatypes.txt

  • The reviewer executed the new code features at least once and checked the results manually

  • The code is covered in an existing or new test case

  • New tests use the Google Test framework

  • The code follows the t8code coding guidelines

  • The code is well documented

  • All function declarations, structs/classes and their members have a proper doxygen documentation

  • All new algorithms and data structures are sufficiently optimal in terms of memory and runtime (If this should be merged, but there is still potential for optimization, create a new issue)

  • If this PR introduces a new feature, it must be covered in an example/tutorial and a Wiki article.

@sandro-elsweijer sandro-elsweijer changed the title added linear geometries for unlinked trees fix issue 353 Jan 26, 2023
@sandro-elsweijer sandro-elsweijer changed the title fix issue 353 Fix issue 353 Jan 26, 2023
@holke
Copy link
Collaborator

holke commented Jan 27, 2023

Sorry i messed up a bit and confused the 353 and 352 branches.
Had to do a force push on this branch to reset my mess.

Base automatically changed from fix-issue_352 to main January 30, 2023 09:56
@sandro-elsweijer
Copy link
Collaborator Author

This should also enable us to work on issue #399

@sandro-elsweijer sandro-elsweijer marked this pull request as ready for review January 30, 2023 12:47
Copy link
Collaborator

@holke holke left a comment

Choose a reason for hiding this comment

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

Looks good.
We currently do not have a test for this.
We do not have any test for reading a .msh file with .brep info i think?
So we should write one.
Minor change is that i dont think the string copying is really necessary and can be done via simple assignment.

src/t8_cmesh/t8_cmesh_readmshfile.cxx Outdated Show resolved Hide resolved
src/t8_cmesh/t8_cmesh_readmshfile.cxx Outdated Show resolved Hide resolved
src/t8_cmesh/t8_cmesh_readmshfile.cxx Outdated Show resolved Hide resolved
@holke holke assigned sandro-elsweijer and unassigned holke Feb 6, 2023
@holke holke marked this pull request as draft February 7, 2023 16:27
@holke
Copy link
Collaborator

holke commented Feb 7, 2023

Converted to draft since #380 was reverted.

@holke holke marked this pull request as ready for review February 27, 2023 14:28
@holke
Copy link
Collaborator

holke commented Feb 27, 2023

@sandro-elsweijer can you please verify whether this is working now (especially on partitioned cmeshes)?
Since Lukas fixed the attribute bug in #444

@sandro-elsweijer sandro-elsweijer removed their assignment Mar 13, 2023
@sandro-elsweijer sandro-elsweijer changed the base branch from main to revert-427-revert-380-fix-issue_352 March 14, 2023 10:10
@sandro-elsweijer sandro-elsweijer marked this pull request as draft March 14, 2023 10:11
@sandro-elsweijer sandro-elsweijer added enhancement Enhances already existing code shouldn't take long Can be resolved in under 30 mins draft Enhance the visibility that this is a draft. labels Mar 17, 2023
@holke
Copy link
Collaborator

holke commented Mar 22, 2023

This is still marked as a draft.
@sandro-elsweijer can this be taken out of draft status?

@holke holke assigned sandro-elsweijer and unassigned holke Mar 22, 2023
@sandro-elsweijer
Copy link
Collaborator Author

This is still marked as a draft.
@sandro-elsweijer can this be taken out of draft status?

@holke No, since it depends on #470. But it is reviewable.

Base automatically changed from revert-427-revert-380-fix-issue_352 to main March 24, 2023 14:50
@sandro-elsweijer sandro-elsweijer marked this pull request as ready for review March 25, 2023 18:35
@sandro-elsweijer sandro-elsweijer merged commit 404cc08 into main Mar 25, 2023
@sandro-elsweijer sandro-elsweijer deleted the fix-issue_353 branch March 25, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
draft Enhance the visibility that this is a draft. enhancement Enhances already existing code shouldn't take long Can be resolved in under 30 mins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define linear geometry for non-linked trees in curved meshes.
2 participants