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

unintended prefix for compartment i #366

Closed
1 of 3 tasks
mihai-sysbio opened this issue Sep 3, 2021 · 2 comments
Closed
1 of 3 tasks

unintended prefix for compartment i #366

mihai-sysbio opened this issue Sep 3, 2021 · 2 comments
Labels
fixed in develop This issue is fixed and pushed to develop branch. Will be closed when fix appears in master branch.

Comments

@mihai-sysbio
Copy link
Member

Description of the issue:

When exporting Human-GEM from yml to xml, the compartment i becomes c_i. It is the only compartment that gets prefixed with c_. The root cause is likely in the lines

RAVEN/io/exportModel.m

Lines 224 to 231 in 800362e

if isfield(modelSBML.compartment,'metaid')
if ~isnan(str2double(model.comps(i)))
EM='The compartment IDs are in numeric format. For the compliance with SBML specifications, compartment IDs will be preceded with "c_" string';
dispEM(EM,false);
model.comps(i)=strcat('c_',model.comps(i));
end
modelSBML.compartment(i).metaid=model.comps{i};
end

Maybe due to the unintended interpretation of i as an imaginary unit?

Reproducing this issue:

Below is a test case that demonstrates the bug:

compartment = 'i';
if ~isnan(str2double(compartment))
compartment=strcat('c_',compartment);
end
disp(compartment)
c_i

System information

The bug is based on the latest release of RAVEN, demonstrated above in Matlab 2019b on macOS.

I hereby confirm that I have:

@edkerk
Copy link
Member

edkerk commented Sep 3, 2021

You're right. Checking whether something is numeric using str2double is very sloppy, this is fixed by changing line 225 as:

        if any(regexp(model.comps(i),'^\d'))

Will make a PR for this asap. Meanwhile, exportModel from #360, together with the extended checkModelStruct from #353 would prevent these issues.

@mihai-sysbio
Copy link
Member Author

mihai-sysbio commented Sep 4, 2021

Will make a PR for this asap. Meanwhile, exportModel from #360, together with the extended checkModelStruct from #353 would prevent these issues.

Thanks @edkerk for the quick action and good advice.

@edkerk edkerk added the fixed in develop This issue is fixed and pushed to develop branch. Will be closed when fix appears in master branch. label Sep 20, 2021
@edkerk edkerk mentioned this issue Jan 13, 2022
4 tasks
@edkerk edkerk closed this as completed Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in develop This issue is fixed and pushed to develop branch. Will be closed when fix appears in master branch.
Projects
None yet
Development

No branches or pull requests

2 participants