Jdv/test fix

[josephdviviano]

• Added test repeats configurable on a per-test basis. This is to avoid runtimes that are > 4 hours.
• Added configurable n_states
• Some linter errors fixed.
• TODOs added.
• A follow up PR will address the allocation of compute to the various tests for maximum efficiency.

Addresses #242

[josephdviviano]

@alexhernandezgarcia @carriepl

have a look at this draft - it's not finished nor linted etc, but it shows a working example for the ccrystals and cube environments.

I don't love the code duplication in common.py but i'm not sure how better to handle that (I was thinking there is maybe a decorator but I'm not sure how to pass the args to it - anyway I think it's not crucial to fix this now).

[alexhernandezgarcia]

@alexhernandezgarcia @carriepl

have a look at this draft - it's not finished nor linted etc, but it shows a working example for the ccrystals and cube environments.

I don't love the code duplication in common.py but i'm not sure how better to handle that (I was thinking there is maybe a decorator but I'm not sure how to pass the args to it - anyway I think it's not crucial to fix this now).

The code repetition you refer to is the lines

if _get_current_method_name() in self.repeats:
    n_repeat = self.repeats[_get_current_method_name()]

in all tests, or something else?

[josephdviviano]

Yes exactly. It's a bit ugly to do this, but not a showstopper.

Joseph Viviano @josephdviviano https://twitter.com/josephdviviano viviano.ca

On Fri, Jan 19, 2024 at 5:34 PM Alex @.***> wrote:

@alexhernandezgarcia https://github.com/alexhernandezgarcia @carriepl https://github.com/carriepl

have a look at this draft - it's not finished nor linted etc, but it shows a working example for the ccrystals and cube environments.

I don't love the code duplication in common.py but i'm not sure how better to handle that (I was thinking there is maybe a decorator but I'm not sure how to pass the args to it - anyway I think it's not crucial to fix this now).

The code repetition you refer to is the lines

if _get_current_method_name() in self.repeats: n_repeat = self.repeats[_get_current_method_name()]

in all tests, or something else?

— Reply to this email directly, view it on GitHub https://github.com/alexhernandezgarcia/gflownet/pull/267#issuecomment-1901234523, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7TL2QYLEK3ZZW34MULYQDYPLYFZAVCNFSM6AAAAABAIQQJDGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBRGIZTINJSGM . You are receiving this because you were assigned.Message ID: @.***>

[alexhernandezgarcia]

I agree. I will be able to sleep ok with this :P

[josephdviviano]

Yeah I'll likely remove this entire file when I go to merge the PR (I need first to fix the dependencies, or CI will continue to crash).

Joseph Viviano @josephdviviano https://twitter.com/josephdviviano viviano.ca

On Wed, Jan 24, 2024 at 1:37 PM Alex @.***> wrote:

@.**** commented on this pull request.

In tests/gflownet/envs/test_tests.py https://github.com/alexhernandezgarcia/gflownet/pull/267#discussion_r1465389923 :

+# def repeattest(times): +# def decorator(func): +# def wrapper(*args, **kwargs): +# for in range(times): +# func(*args, **kwargs) +# return wrapper +# return decorator

Do you want to keep these lines commented out?

— Reply to this email directly, view it on GitHub https://github.com/alexhernandezgarcia/gflownet/pull/267#pullrequestreview-1842121259, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7TL2TDM2OQDMG5VWYJRXDYQFIGRAVCNFSM6AAAAABAIQQJDGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNBSGEZDCMRVHE . You are receiving this because you were mentioned.Message ID: @.***>

[alexhernandezgarcia]

Yeah I'll likely remove this entire file when I go to merge the PR (I need first to fix the dependencies, or CI will continue to crash).

Just as a suggestion, in case it's helpful. If you run it in the Mila cluster, the following should install all you need:

git fetch origin setupenvs
git show setupenvs:mila/setup/setupenv_full_py3.10_cu11.8_torch2.0.1.sh > .tmp.installgflownet.sh
source .tmp.installgflownet.sh ~/scratch/.virtualenvs/gflownet_full_py3.10_cu11.8_torch2.0.1
source ~/scratch/.virtualenvs/gflownet_full_py3.10_cu11.8_torch2.0.1/bin/activate
rm .tmp.installgflownet.sh

[josephdviviano]

Thanks but I think we need a solution that will run on an arbitrary machine

Joseph (Mobile)

On Wed, Jan 24, 2024 at 14:37 Alex @.***> wrote:

Yeah I'll likely remove this entire file when I go to merge the PR (I need first to fix the dependencies, or CI will continue to crash).

Just as a suggestion, in case it's helpful. If you run it in the Mila cluster, the following should install all you need:

git fetch origin setupenvs git show setupenvs:mila/setup/setupenv_full_py3.10_cu11.8_torch2.0.1.sh > .tmp.installgflownet.shsource .tmp.installgflownet.sh ~/scratch/.virtualenvs/gflownet_full_py3.10_cu11.8_torch2.0.1source ~/scratch/.virtualenvs/gflownet_full_py3.10_cu11.8_torch2.0.1/bin/activate rm .tmp.installgflownet.sh

— Reply to this email directly, view it on GitHub https://github.com/alexhernandezgarcia/gflownet/pull/267#issuecomment-1908794235, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7TL2VDRS7JISZEOXWXZDDYQFPJBAVCNFSM6AAAAABAIQQJDGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBYG44TIMRTGU . You are receiving this because you were mentioned.Message ID: @.***>

[alexhernandezgarcia]

Thanks but I think we need a solution that will run on an arbitrary machine

Ok, I thought you were asking about the way for you specifically to install it.

In any case, how is it possible to make it arbitrary if installing torch requires specifying cuda version?

[josephdviviano]

All changes incorporated. I'll merge this up, and then we can do a second pass where we tune the number of repeats / states explored for each env.