Skip to content

Non-mmcs sampling fails #117

Closed
Closed

Description

@hariszaf

I noticed that when using generate_steady_states_no_multiphase() with any method except mmcs, you would get a zero-array of shape equal to the expected one - that's why the tests do not fail.

(base) u0156635@gbw-l-l0074:ext_data$ python 
Python 3.9.5 | packaged by conda-forge | (default, Jun 19 2021, 00:32:32) 
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import dingo
>>> model = dingo.MetabolicNetwork.from_sbml("e_coli_core.xml")
>>> sampler = dingo.PolytopeSampler(model)
>>> samples = sampler.generate_steady_states_no_multiphase()
>>> samples
array([[0., 0., 0., ..., 0., 0., 0.],
       [0., 0., 0., ..., 0., 0., 0.],
       [0., 0., 0., ..., 0., 0., 0.],
       ...,
       [0., 0., 0., ..., 0., 0., 0.],
       [0., 0., 0., ..., 0., 0., 0.],
       [0., 0., 0., ..., 0., 0., 0.]])
  • OS: Ubuntu 24

Activity

  1. added a commit that references this issue on Aug 4, 2025
    6ec7172
  2. AyushDwi commented on Oct 30, 2025

    @AyushDwi

    Hi @hariszaf! I'd like to work on fixing this issue for GSoC 2026.

    I've reproduced the bug using e_coli_core.xml and confirmed that
    non-mmcs methods return all zeros.

    My plan:

    1. Investigate the generate_steady_states_no_multiphase() implementation
    2. Find where the bug occurs
    3. Fix the root cause
    4. Add comprehensive tests to prevent regression

    Can you help me understand the intended behavior of each sampling method?

  3. ChinmayRout9040895625 commented on Dec 1, 2025

    @ChinmayRout9040895625

    Hi @hariszaf! I looked into this issue, and I can reproduce the exact same behaviour:

    1. generate_steady_states_no_multiphase() returns a sample matrix with the correct shape,
    2. but all entries are zeros for every non-MMCS sampling method.
      MMCS sampling works correctly, so the failure seems isolated to the non-MMCS path.
      so based on initial inspection, the problem is likely happening at the Cython/C++ wrapper layer:
    3. the numpy array is being created but never populated during non-MMCS sampling,
    4. or the C++ sampler returns a sample matrix that is not assigned back to the Python variable.
      In contrast, the MMCS path seems to correctly fill the output buffer.

    What i plan to do -

    1. Compare the MMCS wrapper call with the non-MMCS wrapper in volestipy.pyx.
    2. To verify whether the non-MMCS sampler writes to the provided numpy buffer or returns a new matrix.
    3. Add a regression test that checks for non-zero variance .

    Let me know if you would like a PR draft with these changes.

    Thanks!

  4. hariszaf commented on Dec 1, 2025

    @hariszaf
    CollaboratorAuthor

    Hi @ChinmayRout9040895625

    Thanks for the interest on our project.

    Do you still get this error after the cdcb43c commit?

  5. ChinmayRout9040895625 commented on Dec 1, 2025

    @ChinmayRout9040895625

    I haven’t tested it after commit cdcb43c yet – the behaviour I described was from a build before that commit. I will do the changes (including cdcb43c), re-run the non-MMCS example, and report back here whether I can still reproduce the all-zero samples or if it’s fixed now.

  6. ChinmayRout9040895625 commented on Dec 1, 2025

    @ChinmayRout9040895625

    Hi @hariszaf!, thanks for the pointer to cdcb43c.

    I pulled the latest develop (including cdcb43c) and re-ran non-MMCS sampling with generate_steady_states_no_multiphase using:

    model = MetabolicNetwork.from_sbml("ext_data/e_coli_core.xml")
    sampler = PolytopeSampler(model)
    steady_states = sampler.generate_steady_states_no_multiphase(
    method="billiard_walk",
    n=500,
    burn_in=100,
    thinning=2,
    )
    I still get a matrix with the correct shape, but all entries are zero (I checked with np.any(np.abs(steady_states) > 1e-12) which returns False).
    So the generate_steady_states_no_multiphase is calling HPolytope.generate_samples(...) and directly using its return value, so the issue now seems to be in the implementation of HPolytope.generate_samples rather than PolytopeSampler.py.

    I’m happy to dig into the Cython / volesti side if you’d like some help.

  7. vissarion commented on Dec 9, 2025

    @vissarion
    Member

    @ChinmayRout9040895625 sorry I cannot reproduce your issue. However, if the method is "mmcs" then indeed dingo returns a zero matrix. This is fixed in #122. See the tests in #122 for more details.

  8. ChinmayRout9040895625 commented on Dec 15, 2025

    @ChinmayRout9040895625

    Thanks a lot for checking this and for the clarification!
    That makes sense — I see now that the zero-matrix behaviour was specific to using "mmcs" via the non-multiphase path, and it’s great that this is already addressed in #122 (the tests there are very helpful as well).

    Thanks again for the quick investigation and fix.
    Please let me know if there’s any other issue, testing, or follow-up where I could help — I’d be happy to contribute

  9. vissarion commented on Jan 9, 2026

    @vissarion
    Member

    OK I am closing this one @hariszaf @ChinmayRout9040895625 feel free to reopen or open another issue if you find something that is not working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions