search

Cantera / enhancements

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

Deprecate (or remove?) legacy Matlab Toolbox in Cantera 3.1 #196

Closed ischoegl closed 3 months ago

ischoegl commented 4 months ago

Abstract

The "experimental" MATLAB toolbox is making progress (see #32, #177 and associated issues). At the same time, the "legacy" toolbox does not support many of the newer Cantera features (example: C++ Solution) and uses legacy implementations instead that prevent simplifications and thus creates added maintenance efforts.

Motivation

Describe the need for the proposed change:

Possible Solutions

All that is needed to do in for the legacy toolbox is to trigger deprecation warnings. The bulk of the work is in making the (currently experimental) re-implementation of the MATLAB toolbox ready for final release, i.e. #177. Tagging @rwest, @ssun30 for comments.

References

rwest commented 4 months ago

Thanks Ingmar. I guess #177 (Complete the new toolbox) is needed as well as this issue #196 (Deprecate the old toolbox). The former seems to be a case of needing tests, deployment, and instructions, but maybe is best discussed on that thread instead.

ischoegl commented 4 months ago

Thanks Ingmar. I guess #177 (Complete the new toolbox) is needed as well as this issue #196 (Deprecate the old toolbox). The former seems to be a case of needing tests, deployment, and instructions, but maybe is best discussed on that thread instead.

Correct - changes to the new toolbox should be discussed in #177. At the same time, #177 has no impact on the deprecation cycle, which is why I opened this separate enhancement request. At least from my perspective, I am in favor of deprecating the old toolbox in the next release (Cantera 3.1), regardless of the state of #177.

speth commented 4 months ago

I wonder if in this case there's a more efficient way of deprecating the old toolbox.

When we deprecate individual methods / classes within a given interface, the purpose of keeping the old methods in place with deprecation warnings is to give users a chance to update their code so it can work with both the current and next Cantera release. However, in the case of the new Matlab toolbox, that's not an option -- user code will have to be updated in ways that are not backwards compatible with the old toolbox.

As a result, I don't think there's much value in keeping the old Matlab toolbox in the 3.1 release and having it issue deprecation warnings every time you use it. Instead, I would suggest that we can update the downloads/installation sections of the website to note that the 3.0 release is the last version of the old toolbox, and that a future Cantera version (hopefully, but not necessarily 3.1) will include a stable version of the new toolbox. Anyone who needs the current toolbox can just stick to Cantera 3.0 until they're ready to upgrade. This approach would allow us remove the old toolbox from the repository immediately, rather than needing to wait for another Cantera release.

ischoegl commented 4 months ago

@speth ... thanks for your comment. I guess I'd be :+1: for complete removal as well. I don't think that there will be any game-changers in 3.1 that would be usable from within the legacy MATLAB toolbox anyways.

ischoegl commented 4 months ago

Alright. Based on the reaction, I created a PR to remove the legacy MATLAB toolbox - see Cantera/cantera#1670.

In a nutshell: ~15k lines that are not under CI would be removed, which is pretty substantial.