Skip to content

Optimize #902 remove parfor #968

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
Merged
jan-petr merged 2 commits into from
Dec 2, 2021
Merged

Optimize #902 remove parfor #968

jan-petr merged 2 commits into from
Dec 2, 2021

Conversation

@jan-petr
Copy link
Contributor

@jan-petr jan-petr commented Dec 2, 2021

Linked issue

#902

@jan-petr jan-petr changed the base branch from main to develop December 2, 2021 16:40
@jan-petr jan-petr linked an issue Dec 2, 2021 that may be closed by this pull request
Do not use parfor in xASL_Iteration or elsewhere #902
Closed
@jan-petr
Copy link
Contributor Author

jan-petr commented Dec 2, 2021

Shall we remove code on line 182 as well?

@MichaelStritt MichaelStritt added the optimization Ensure that code runs faster with unchanged functionality label Dec 2, 2021
MichaelStritt
MichaelStritt approved these changes Dec 2, 2021
View reviewed changes
Copy link
Contributor

@MichaelStritt MichaelStritt left a comment

Choose a reason for hiding this comment

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

👍

@MichaelStritt
Copy link
Contributor

MichaelStritt commented Dec 2, 2021

Shall we remove code on line 182 as well?

Hm... not sure. Let's not change too much to keep everything stable before the release.

HenkMutsaerts
HenkMutsaerts requested changes Dec 2, 2021
View reviewed changes
Copy link
Member

@HenkMutsaerts HenkMutsaerts left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice, what about the other two occurrences? Do we leave them in?

cat_surf_epivolsurf
zspm_dcm_fit

Nobody uses these, so safe to replace the parfor with for here to be sure Matlab never checks for this toolbox.

Then we can close the other issue as "duplicate".

@jan-petr
Copy link
Contributor Author

jan-petr commented Dec 2, 2021

zspm_dcm_fit

Nice, what about the other two occurrences? Do we leave them in?

cat_surf_epivolsurf zspm_dcm_fit

Nobody uses these, so safe to replace the parfor with for here to be sure Matlab never checks for this toolbox.

There it is already commented out. No need to remove.

Then we can close the other issue as "duplicate".

Which other issue? #888 this is already closed...

@jan-petr jan-petr merged commit bfa6b83 into develop Dec 2, 2021
@MichaelStritt MichaelStritt deleted the optimize-#902_RemoveParfor branch December 3, 2021 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@HenkMutsaerts HenkMutsaerts HenkMutsaerts approved these changes

+1 more reviewer

@MichaelStritt MichaelStritt MichaelStritt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

optimization Ensure that code runs faster with unchanged functionality

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Do not use parfor in xASL_Iteration or elsewhere

4 participants

@jan-petr @MichaelStritt @HenkMutsaerts