search

Cantera / enhancements

Repository for proposed and ongoing enhancements to Cantera
11 stars 5 forks source link

Complete the experimental Matlab toolbox. #177

Open ssun30 opened 1 year ago

ssun30 commented 1 year ago

Abstract

Implement the currently missing features in the experimental Matlab interface.

Motivation

Describe the need for the work being done:

Description

References

Links to a development branch in your fork of the Cantera repository, Pull Requests, GitHub Issues, Users' Group topics, or other relevant material.

speth commented 1 year ago

Without automated installation there is no easy way to adding testing into the CI workflow.

I think introducing a test suite that can be run through GitHub Actions should be the top priority for this toolbox. I'm not sure why automating installation would be necessary for this -- one of the best features of using calllib is that there is nothing to compile on the Matlab side -- all you need is a copy of cantera_shared.so and the header files.

If you can get started on the test suite itself, I'd be happy to help with the GitHub Actions integration.

ischoegl commented 1 year ago

@ssun30 ... thanks for posting. Is this work in progress or a feature request?

ssun30 commented 1 year ago

@ssun30 ... thanks for posting. Is this work in progress or a feature request?

This is a work in progress. Sorry I couldn't find where to change the label of this issue.

ischoegl commented 1 year ago

Fantastic… changed the label for you.

rwest commented 1 year ago

We think @ssun30's new test suite has found some bugs (hurrah - it's working). Should the bug fixes be a separate pull request? How is it best structured and broken up? (We'd probably like to avoid reviewing another gazillion-line pull request.)

ischoegl commented 1 year ago

We think @ssun30's new test suite has found some bugs (hurrah - it's working). Should the bug fixes be a separate pull request? How is it best structured and broken up? (We'd probably like to avoid reviewing another gazillion-line pull request.)

🎉 … my 2 cents would be to create PR’s that collect those bugs, using ‘draft’ mode and the ‘ready for review’ check box to mark progress. That way, we should be able to review a small number of PR’s without getting to an excessive line count.

speth commented 1 year ago

As a starting point, I'd say it would be nice to begin with a PR that introduces some portion of the test suite and a way of running it (assuming that's in place) without getting into too many bug fixes in the first PR. If there are tests that currently fail, you can mark them as such so they don't cause the whole test suite or CI to indicate failure.

From there, you can create new PRs that both introduce new test code and bug fixes needed for those tests to succeed. This doesn't need to be "one bug per PR", but it would be good to try to limit the scope of any individual PR, be that in terms of number of lines of code to review, type of bug being fixed, or number of files/functions impacted.

ischoegl commented 1 year ago

As a starting point, I'd say it would be nice to begin with a PR that introduces some portion of the test suite and a way of running it (assuming that's in place) without getting into too many bug fixes in the first PR. […]

… on second thought that would indeed be nice. I assume you’re using Mathworks’ testing frameworks … in which case there likely isn’t much required to make this run? Initial tests can be brutally simple. As long as the approach is compatible with the GH Matlab action or similar - which probably needs to be tested before investing too much work - simple instructions may suffice for starters.

ischoegl commented 1 year ago

@ssun30 ... fwiw, I just refactored the Func1 functor interface (see Cantera/cantera#1513), which should make things a lot simpler going forward. Once merged, this will make all Func1 specializations accessible from clib, which should help with a resolution of Cantera/cantera#567 in the new interface.

ischoegl commented 5 months ago

@ssun30 … thank you for all your work on this thus far. Given that we released 3.0 with the experimental version of this MATLAB toolbox, what are the remaining steps for the final release?

ssun30 commented 5 months ago

@ssun30 … thank you for all your work on this thus far. Given that we released 3.0 with the experimental version of this MATLAB toolbox, what are the remaining steps for the final release?

Hi Ingmar, sorry I've been busy working on my dissertation proposal and class projects towards the end of last year. Now I have time to work on the toolbox again. I think the remaining tasks for the final relase include:

ischoegl commented 5 months ago

@ssun30 … thank you for all your work on this thus far. Given that we released 3.0 with the experimental version of this MATLAB toolbox, what are the remaining steps for the final release?

Hi Ingmar, sorry I've been busy working on my dissertation proposal and class projects towards the end of last year. Now I have time to work on the toolbox again. I think the remaining tasks for the final relase include:

  • Complete the test suite and add these to the CI (In Progress)
  • Clean up the documentations.
  • Incorporate the installation of the toolbox into the SCons scripts.
  • Build the msi package for Windows.

Thank you, @ssun30! That sounds great - it appears to be reasonably close to the finishing line. Please let us know in case you should need any assistance with the finishing touches. I'm definitely looking forward to a release in a 'stable' version.

FWIW: regarding documentation, the current builds for 3.1 alpha use sphinx>=7.2,<8 (for whatever is built using GH Actions)