Skip to content

MATLAB:mpath:PathAlterationNotSupported / Modifying the search path is not supported by MATLAB Compiler using MAtlab 2019b #3507

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

Closed
servoz opened this issue Sep 19, 2022 · 13 comments · Fixed by #3509

Comments

@servoz
Copy link
Collaborator

servoz commented Sep 19, 2022

It seems modifying the matlab path using addpath() is not likely to be supported at any point after MATLAB MCR_R2018b.
However, nipype use addpath() command in the matlab script generated .
So the easy fix is to stick with R2018b.
But it's a real problem not to be able to use newer MCRs in the future.
Can you remove the addpath() command when generating the script ? Or is there another way to use MCR after R2018b?

@DimitriPapadopoulos
Copy link
Contributor

I'd like to add that upcoming versions of SPM12 will likely be built with R2021b (MCR v911). If so, downgrading to a previous versions of the MCR might not be possible. The only log term fix is ti get rid of addpath().

@effigies
Copy link
Member

Do y'all know what a fix would be? We can obviously remove the line, but we'd need to confirm that it doesn't break workflows.

@DimitriPapadopoulos
Copy link
Contributor

Keeping it breaks workflows when using recent versions of MCR which is required by Ubuntu 20.04 and later.

I'd simply remove it.

@effigies
Copy link
Member

I understand that it is causing forward compatibility problems. Before removing it, we need to understand what problem it was solving and make sure the problem remains solved with the final change.

At worst, we should be able to detect the Matlab version and drop it for more recent versions.

@DimitriPapadopoulos
Copy link
Contributor

We haven't found any problem it could solve. It looks like addpath() had been introduced by 078619c without any explanation.

@servoz
Copy link
Collaborator Author

servoz commented Sep 20, 2022

The tests we have done on our side seem to indicate that with the MCR, things work correctly by simply commenting out the addpath() command.
In my opinion, when using the MCR, addpath() is unnecessary as the path to spm standalone is known elsewhere.

I think it is useful when using MATLAB as we may want to define where SPM is located.

I suggest as a fix to change here:

prescript.append("addpath('%s');\n" % path)
with
prescript.append("if (isdeployed==false) addpath('%s'); end \n" % path)

I have not tested this fix but if you wish we can test it with an MCR > R2018b?

However, you are right to test on your side if this does not lead to a regression that we did not think of!

@servoz
Copy link
Collaborator Author

servoz commented Sep 20, 2022

Sorry for the typo!
The effective fix is (with a comma after end):
prescript.append("if (isdeployed==false) addpath('%s'); end,\n" % path)

I just tested it.

As expected, if we remove the following two lines from here:

for path in paths :
    prescript.append("addpath('%s');\n" % path)

This works well with MCR but not with MATLAB (if SPM was not already in the matlab path).

By using :

for path in paths:
    prescript.append("if (isdeployed==false) addpath('%s'); end,\n" % path)

everything works fine with MCR and MATLAB (even if SPM was not already in the MATLAB path).

Tested with MCR_R2018b and MATLAB R2020b.

@effigies
Copy link
Member

Sounds great to me. Would you like to open a PR?

@servoz
Copy link
Collaborator Author

servoz commented Sep 20, 2022

As you like! I do it quickly.

@DimitriPapadopoulos
Copy link
Contributor

@servoz I have suggested merge request #3509.

@DimitriPapadopoulos
Copy link
Contributor

But I can put you as author of the code if you want to.

Note the if ~(ismcc || isdeployed) from https://mathworks.com/help/compiler/isdeployed.html.

@servoz
Copy link
Collaborator Author

servoz commented Sep 20, 2022

Oh great ! Thanks @DimitriPapadopoulos !

@servoz
Copy link
Collaborator Author

servoz commented Sep 20, 2022

no no it's fine like this, I don't want to be an author!

denisri added a commit to brainvisa/casa-distro that referenced this issue Oct 27, 2022
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 a pull request may close this issue.

3 participants