pmgbergen / porepy

Python Simulation Tool for Fractured and Deformable Porous Media
GNU General Public License v3.0
251 stars 88 forks source link

Delete the method _parse_other in the Ad operator class #1180

Closed keileg closed 2 months ago

keileg commented 5 months ago

The method pp.ad.Operator._parse_other() is almost surely never used in practice; rather it is a remnant of an older approach to the parsing. We should verify that it is actually not used, and purge if possible. This should not be a major task.

This will be a preparatory step towards a full rework of the Ad operator parsing. Though the purge can be done together with a larger rewrite, it is preferable to do it before, so that the code is a bit cleaner when we move to the larger part.

mikeljordan commented 2 months ago

Quick Look at the Operator class: The _parse_other() method is only used through implicit calls in the operator overloading methods (e.g., add, mul, etc.). This is a legacy approach and should be reviewed for potential refactoring or removal if the operator overloads methods are not used.

keileg commented 2 months ago

If the original observation, that this method is not used, is wrong, I suggest we close the issue.

mikeljordan commented 2 months ago

On a final note, the method _parse_other() cannot be removed because of the implicit calls to it by the overloading methods within the operator class which are inherently used whenever there are operator arithmetics elsewhere in the codebase. Removing it would mean code duplications (dumping its implementations where it's called), which I think deviate from what we want to achieve.