Node difference for abscissa in Primal/Dual Integrals

ds4dm / ecole

Extensible Combinatorial Optimization Learning Environments

https://www.ecole.ai

BSD 3-Clause "New" or "Revised" License

319 stars 68 forks source link

Node difference for abscissa in Primal/Dual Integrals #268

Open jdumouchelle opened 2 years ago

jdumouchelle commented 2 years ago

Pull request checklist

[ ] I have opened an issue to discuss the proposed changes: Fix #XX.
[x] I have modified/added tests to cover the new changes/features.
[x] I have modified/added the documentation to cover the new changes/features.
[x] I have ran the tests, checks, and code formatters.

Proposed implementation

Implementation of primal, dual, and primal dual integrals with respect to number of nodes.

From the perspective of the user, one can simply pass use_nnodes=True into the constructor of any of the integral reward functions.

AntoinePrv commented 2 years ago

Thanks @jdumouchelle. The code looks good to me, however I'm wondering if we should make this future-proof. Today someone asks to use the number of nodes in the abscissa, next time it's the LP iterations... Perhaps we could add a callback, just like we did for bound_function (and add a some aliases to make it easier to use).

Meta level

At a more abstract level, I believe Integral should in fact be more of an operator. In the same way that we can sum two reward functions, we could make it so that Integral can be computed for any two reward function (one for the abscissa and one for the ordinate).

jdumouchelle commented 2 years ago

Yes I agree. It did come across my mind, but it will likely require a decent amount of time. Perhaps we can merge this for now so it can be used by Aaron, open an issue for a more general integral reward, and discuss it in there. After that, I will work on when I have more time.