cavalab / brush

An interpretable machine learning library
http://cavalab.org/brush/
GNU General Public License v3.0
2 stars 0 forks source link

tests for weight optimization #14

Closed lacava closed 1 year ago

gAldeia commented 1 year ago

Hello William,

I have some questions regarding coefficient optimization.

As far as I understood, if the node has is_weighted=true, then the weights are applied to each of its children. A weighted sqrt would be equivalent to sqrt(w*<child>).

I was trying to somehow add and adjust external weights as well. My question is: in order to have an external weight (such as w2*sqrt(w*<child>)), then the parent of sqrt should be a node with is_weighted set to true as well? Or the weight could explicitly be created by multiplying a constant node with sqrt(...)?

Also, do constant nodes have its values fixed, or they can be adjusted through optimization as well? I have tried to create constant nodes manually by using your json parser, but I didn't managed to get it working.

Regarding organization:

lacava commented 1 year ago

Hello William,

I have some questions regarding coefficient optimization.

As far as I understood, if the node has is_weighted=true, then the weights are applied to each of its children. A weighted sqrt would be equivalent to sqrt(w*<child>).

Yes that's right! This is a design choice that we might want to revisit. I set it up this way to make backpropagation easier, but now that ceres is handling weight updates, we could put the weights anywhere.

I was trying to somehow add and adjust external weights as well. My question is: in order to have an external weight (such as w2*sqrt(w*<child>)), then the parent of sqrt should be a node with is_weighted set to true as well? Or the weight could explicitly be created by multiplying a constant node with sqrt(...)?

Yes, that's correct.

Also, do constant nodes have its values fixed, or they can be adjusted through optimization as well? I have tried to create constant nodes manually by using your json parser, but I didn't managed to get it working.

I have not spend enough time on the Constants, they need to be developed. Let me know what is happening. We should probably prioritize getting them working.

Regarding organization:

  • Should I test every operator? (if so, is that ok to populate /docs/examples/datasets/ with generated files, or it is better to have it hardcored in the tests?)
  • I'm thinking about separating the optimization tests into an individual file, is that ok?

Maybe it's better to hard code the tests rather than have a lot of data files.

Also, if it's easier you can now write the tests in Python using the pybind11 bindings: https://github.com/cavalab/brush/blob/4b332426c1666c56834fa855956fc0b0d8e643da/tests/python/test_program.py#L52

gAldeia commented 1 year ago

The python binding installation is working like a charm!! I am planning to do the tests in python (in the future I can replicate them for the C++ version).

I managed to get nose to work, but that required manual configurations. I found that nose is no longer maintained. Its successor, nose2, encourages migrating to pytest (I have used this one in the past, so I am familiar with it).

I am going to rewrite previous tests, then implement new ones with the pytest testing framework.

lacava commented 1 year ago

Use pytest, not nose!

On Wed, Mar 29, 2023, 8:19 AM Guilherme Aldeia @.***> wrote:

The python binding installation is working like a charm!! I am planning to do the tests in python (in the future I can replicate them for the C++ version).

I managed to get nose to work, but that required manual configurations. I found that nose is no longer maintained. Its successor, nose2, encourages migrating to pytest (I have used this one in the past, so I am familiar with it).

I am going to rewrite previous tests, then implement new ones with the pytest testing framework.

— Reply to this email directly, view it on GitHub https://github.com/cavalab/brush/issues/14#issuecomment-1488499734, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSGCGWWJLHEIZESY47FRYDW6QSGTANCNFSM6AAAAAAQAVFTS4 . You are receiving this because you were assigned.Message ID: @.***>

lacava commented 1 year ago

hey @gAldeia , I made a change to reorder the constants so that they are applied to the outside of the operators. It's on this branch; https://github.com/cavalab/brush/tree/reorder_constants

I think we should go with that going forward. Could you incorporate this into the tests you've written?

gAldeia commented 1 year ago

Hello @lacava! I think this modification is clear to me. Function nodes still have weights, but terminal nodes can have coefficients now. I will work on adjusting the tests!

gAldeia commented 1 year ago

Hi @lacava! During our meeting, I mentioned that I wasn't able to build the docs.

I think this is a problem on my machine, since the GitHub job managed to successfully compile the docs and execute the tests.