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

remove loop in savemphtxt #50

Closed
wants to merge 3 commits into from
Closed

Conversation

brainwatcher
Copy link

as title discription. The fprintf function in for large loop takes too long time.

@fangq
Copy link
Owner

fangq commented Jul 22, 2020

thanks for the patch - unfortunately I can't accept this one. the changes must be local and minimal, you should also not to remove the help info and the original author info.

if you are interested in revising this patch, please restore most of the changes (including white spaces), and keep the modifications to the least number of lines, and ensure no change to the functionality.

@brainwatcher
Copy link
Author

I had added the author info back and committed again.

savemphtxt.m Outdated
@@ -1,5 +1,4 @@
function savemphtxt(node, face, elem, filename)
%
Copy link
Owner

Choose a reason for hiding this comment

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

can you restore this empty line? without it, matlab has different behaviors

savemphtxt.m Show resolved Hide resolved
savemphtxt.m Outdated
for i = 1:n_face
fprintf(fp,'%d %d %d\n',face(i,1),face(i,2),face(i,3));
end
% for i = 1:n_face
Copy link
Owner

Choose a reason for hiding this comment

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

please remove these commented lines if they are no longer needed. git keeps track of all edit history, so no need to keep those in the code.

@brainwatcher
Copy link
Author

I missed the meshreorient line because my iso2mesh version is rather old. And I had rearranged my file. How about the third committed file?

@fangq fangq closed this in 238c987 Sep 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants