Remove unused functions

[apozas]

I have identified the following functions that are defined but never used:

fast_npa.py: nb_sort_lexorder, nb_op_lessthan_op, nb_op_commuteswith_op2, commuting, mon_times_mon, nb_apply_substitutions.

general_tools.py: mul, factorize_monomials, atomic_is_knowable_memoized, label_knowable_and_unknowable

Do I have the permission to eliminate all these functions? If for some not, why not? Note that we can eliminate them from main, but keep them in devel if necessary (please let me know such cases). Either if you agree or disagree, I'd appreciate an answer so I can proceed as necessary.

[eliewolfe]

I'm fine for you to delete all unused functions, ANYWHERE. However, I believe that is_physical in general_tools.py is used in the InternalAtomicMonomial class, see the import of it in monomial_classes.py

[apozas]

Good catch. It turns out that my editor refuses to search in monomial_classes.py and monomial_utils.py.

[apozas]

@eliewolfe, I believe that the functions is_knowable_q_split_node_check and rectify_fake_setting_atomic_factor in InflationProblem.py are not used anywhere. Am I correct, and thus I have the permission to remove them from main? Do you want me to remove them from devel as well? If the answer is no, would you mind telling me where they are used (so I can debug what's going on with my environment) and write their documentation strings, in the devel branch? Also, if they are to be kept, I would start their names with an _. Do you agree?

[eliewolfe]

No -- those are quite critical, please do not delete! They are used in InflationSDP, for me lines 107 and 108:

        self.is_knowable_q_split_node_check = self.InflationProblem.is_knowable_q_split_node_check
        self.rectify_fake_setting_atomic_factor = self.InflationProblem.rectify_fake_setting_atomic_factor

and then furthermore in line 210

    def atomic_knowable_q(self, atomic_monarray: np.ndarray) -> bool:
        first_test = is_knowable(atomic_monarray)
        if not first_test:
            return False
        else:
            return self.inflation_aware_knowable_q(atomic_monarray)

which is relevant to monomial_classes.py, see line 37

self.knowable_q = self.is_zero or self.is_one or self.sdp.atomic_knowable_q(self.as_ndarray)

and line 40

self.rectified_ndarray = np.asarray(self.sdp.rectify_fake_setting_atomic_factor(np.take(self.as_ndarray, [0, -2, -1], axis=1)), dtype=int)

[apozas]

I see, good, thanks. Then, could you write the corresponding documentation strings?

[eliewolfe]

Doc strings added. I am ok to start their names with _; I'll leave that edit to you.

[apozas]

Thanks a lot! I just pushed the final changes I wanted to do. Note that I renamed rectify_fake_setting_atomic_factor by the simpler and yet (I think) accurate rectify_fake_setting, which I left as public because I could imagine people wanting to know in an easy way what is a given private setting. In a similar spirit, what is the motivation behind the split_node_check in _is_knowable_q_split_node_check? In fact I have a similar question in #54 regarding split_node_model.

[eliewolfe]

Can we close this, @apozas ?

[apozas]

I'd rather keep it open until right before the release. We seem to keep adding/deleting/rewriting functions, and we need a finalized code to be able to reliably know what stays and what leaves.

[apozas]

After going through all the code, I think we can finally close this one.