SBML import renames colliding user parameters to param_<id>, and the overrides filter then hides them

Open
  • Is something wrong?
#2,044 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
52/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Quiet
Tech stack
java
Domain
backend

Research direction

Start in SBMLImporter where ReservedParamPrefix (param_) is prepended to colliding SBML ids, then read MathOverrides.getFilteredConstantNames() to see why param_-prefixed names are dropped from the overrides table. Use #2043 (which fixed the substring collision but left this rule) as the reference for scope and for tests covering name filtering. Done means genuine user parameters are visible and overridable while preserving the display name where feasible, with a plan for models already saved under param_-prefixed names — note the issue leaves the rename-vs-scope decision open, so confirm direction with maintainers.

Written by the indexing model from the issue text.

Description

Split out of #2043 rather than folded into it, since it is a different cause with a different blast radius.

What happens

SBMLImporter renames an imported parameter to param_<id> when its id collides with a reserved symbol but its value differs — i.e. when the importer has already decided it is not the reserved symbol:

private static final String ReservedParamPrefix = "param_";
...
// if the value is different from the value of the reserved symbol, we consider it to be
// something else and we add ReservedParamPrefix
vcParamName = ReservedParamPrefix + sbmlParamId;

So by construction these are genuine user parameters. They are renamed anyway.

Two costs

The user no longer sees the name they wrote. A parameter the author called _R_ in their SBML shows up in VCell as param__R_. Math symbols should nominally be the same as the biological symbols so the end user is not confused; a rename to dodge a collision trades the user's name away to solve an internal problem.

They cannot be overridden. MathOverrides.getFilteredConstantNames() drops anything matching startsWith("param__"), so these parameters are silently absent from the math overrides table — the same user-visible symptom as #2043, reached by a different route. #2043 fixed the substring collision and deliberately left this rule alone.

Worth deciding

  • Is the rename needed at all? The collision is between an SBML id and a VCell reserved symbol; a scoped or mapped resolution would keep the user's name.
  • If some rename is unavoidable, should the overrides filter still hide the result? Hiding a user parameter is the part that reads as a bug regardless of what the name looks like.
  • Any change here has to consider models already saved with param_-prefixed names.

Not urgent

No one has reported it — it was found while diagnosing #2043. Filing so the reasoning is not lost in a code comment.

🤖 Generated with Claude Code

https://claude.ai/code/session_01D71LBYmQNf5J94wPqr81Jx

Dominant language
Java
Stars
140
Forks
35
Avg merge
11h 28m
Merged PRs (30d)
19

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from virtualcell/vcell

All issues in virtualcell/vcell

Similar issues

More Java issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.