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

amiwrap fails in R2018a #307

Closed
thomassligon opened this issue May 23, 2018 · 9 comments
Closed

amiwrap fails in R2018a #307

thomassligon opened this issue May 23, 2018 · 9 comments

Comments

@thomassligon
Copy link
Contributor

I don't see a connection to issue #303, so I'm filing a separate issue. I am using MATLAB R2018a on Windows 10 64-bit.
Amiwrap fails with the following error messages:

Generating model struct ...
Error using sym>convertChar (line 1448)
Character vectors and strings in the first argument can only specify a variable or number. To evaluate character vectors and strings representing symbolic
expressions, use 'str2sym'.

Error in sym>tomupad (line 1214)
        S = convertChar(x);

Error in sym (line 211)
                S.s = tomupad(x);

Error in amimodel/makeSyms (line 121)
        this.sym.Jy(iy) = sym(['0.5*log(2*pi*sigma_y_' num2str(iy-1) '^2) + 0.5*((y_' num2str(iy-1) '-my_' num2str(iy-1) ')/sigma_y_' num2str(iy-1) ')^2']);

Error in amimodel (line 163)
                        AM.makeSyms();

Error in amiwrap (line 111)
        model = amimodel(symfun,modelname);

Error in testAmiWrap (line 2)
    amiwrap('ASTHMA_V40_M07', 'ASTHMA_V40_M07_syms', exdir,false);

testAmiWrap.zip

My MATLAB version is

>> ver
-----------------------------------------------------------------------------------------------------
MATLAB Version: 9.4.0.857798 (R2018a) Update 2
MATLAB License Number: 40322537
Operating System: Microsoft Windows 10 Home Version 10.0 (Build 17134)
Java Version: Java 1.8.0_144-b01 with Oracle Corporation Java HotSpot(TM) 64-Bit Server VM mixed mode
-----------------------------------------------------------------------------------------------------
MATLAB                                                Version 9.4         (R2018a)
Global Optimization Toolbox                           Version 3.4.4       (R2018a)
Optimization Toolbox                                  Version 8.1         (R2018a)
Parallel Computing Toolbox                            Version 6.12        (R2018a)
Statistics and Machine Learning Toolbox               Version 11.3        (R2018a)
Symbolic Math Toolbox                                 Version 8.1         (R2018a)

```A test program has been attached.
FFroehlich pushed a commit that referenced this issue May 23, 2018
@FFroehlich
Copy link
Member

FFroehlich commented May 23, 2018

MATLAB changed/removed an unsupported feature that we exploited for some of the symbolic calculations. Can't really blame them for that, but there currently is no easy fix.

Marking this as wontfix as long as there is no volunteer to look into this. Starting point to fix this is https://github.com/ICB-DCM/AMICI/blob/902b27b4c7346b6e3e2bcec027d929b6b4665562/matlab/%40optsym/optsym.m#L31

@thomassligon
Copy link
Contributor Author

As I mentioned offline, I don't understand the connection between this issue #303 and issue #279 (swig interface), so I don't understand why this should be "won't fix".
Currently, I am trying to debug module makeSyms.m, in hopes of finding a fix for the problem. Line 121 is

this.sym.Jy(iy) = sym(['0.5*log(2*pi*sigma_y_' num2str(iy-1) '^2) + 0.5*((y_' num2str(iy-1) '-my_' num2str(iy-1) ')/sigma_y_' num2str(iy-1) ')^2']);

For the first iteraction (iy==1), this can be resolved as
this.sym.Jy(iy) = sym('0.5*log(2*pi*sigma_y_0^2) + 0.5*((y_0-my_0)/sigma_y_0)^2');

However, when we reach this line, the variables
y_0, my_0. sigma_y_0
are not defined, leading to the error.
I can't find these variables anywhere in makeSyms.m. Where should they be defined?

@FFroehlich
Copy link
Member

As already mentioned, there is no connection between #303 and #279.

You also seem to be using old code, in the newest AMICI commit the line you mentioned has been modified and the issue you are tying to fix is no longer present:
https://github.com/ICB-DCM/AMICI/blob/96100f83a7d0f9dbe0ebef10ba74a5ffd1be9c44/matlab/%40amimodel/makeSyms.m#L129

@thomassligon
Copy link
Contributor Author

changed to betterSym

@thomassligon
Copy link
Contributor Author

Now I am testing in parallel in R2017b (which works) and R2018a.
In makeSyms line 121, replacing sym by str2Sym (or maybe betterSym) fixes the problem. Testing in R2017b shows that the variables y_0, my_0 and sigma_y_0 do not need to be defined at this point, so this looks good.

But then there is a problem in getSyms line 201

tmpxdot = sym(char(optimize(end)));

In this case, changing sym to str2sym does not fix the issue, but leads to another error. Note that optimize(end) starts like this:

array(1..268, 1..1, (1, 1) = t2 + t4 - var_x_0*var_p_11

str2sym does not know how to deal with array(1..268,

What does it mean? Does this line need some kind of string manipulation?

@FFroehlich
Copy link
Member

Hey tom, yes this is the piece of code that I was referring to earlier.

optimize(end) previously yielded the code with intermediate variables and

exprs = arrayfun(@(x) children(x),optimize(1:(end-1)),'UniformOutput',false); % extract symbolic variable
                S.subs = {2};
                S.type='()';
                C = cellfun(@(x) subsref(x,S),exprs,'UniformOutput',false); % get second element
                this.sym = [C{:}]; % transform cell to matrix
                S.subs = {1};
                C = cellfun(@(x) subsref(x,S),exprs,'UniformOutput',false);
                temps = [C{:}]

yielded the definitions of intermediate terms. This is no longer the case due to changes to mupadmex('symobj::optimize',obj.s).

This code was originally take from https://www.mathworks.com/help/symbolic/ccode.html , which should still do similar things (See Section "Write Optimized C Code to File with Comments") this is probably a good starting point to figure out how to adapt the code.

@thomassligon
Copy link
Contributor Author

First a quick remark about the relevance. The decision wontfix means that amiwrap is broken in R2018a and all future versions. What can AMICI do without without amiwrap? This has nothing to do with Windows - it is a problem with MATLAB R2018a on all platforms.

Now to the code. "how to adapt the code" would require understanding what it does. I began by searching for documentation of symobj::optimize, but didn't find anything anywhere. Then I discovered generate::optimize in one place, and that says very little. I tried it out, and the results look a lot like symobj::optimize, but they are not quite the same. I hope that I'm just overlooking something because, if MATLAB doesn't document something, then they have no reason to support it in the future, so coding to it is dangerous.

With that, I can see that fixing makeSyms line 121 is easy, but getSyms line 203 is very complex, and I don't have a fast way to fix it.

@FFroehlich
Copy link
Member

I know, I currently do not have the capacity to further support the matlab interface. I do see the relevance, but it would be great to see some people who are going to continue working with the matlab interface to step up and fix this problem.

As previously mentioned the way we used symobj::optimize was not documented, so we had to expect that mathworks might change something at some point.

I agree that fixing getSyms line 203 is quite involved and I also do not have a quick way to fix it. This is the reason why it currently is marked as wontfix

@dweindl
Copy link
Member

dweindl commented Mar 9, 2021

By now, the documentation clearly states that we require Matlab<R2018a. Therefore, I think we can close this issue.

@dweindl dweindl closed this as completed Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants