Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Future release (1.4.x or 1.5.0 or 2.0) #1350

Open
18 tasks
shuds13 opened this issue Jul 15, 2024 · 4 comments
Open
18 tasks

Future release (1.4.x or 1.5.0 or 2.0) #1350

shuds13 opened this issue Jul 15, 2024 · 4 comments
Assignees
Labels

Comments

@shuds13
Copy link
Member

shuds13 commented Jul 15, 2024

Target date: TBC

Question: Should gen_on_manager becomes default (what about non-persistent gens). This is very breaking hence might be for 2.0.

Canceled / Put on hold:

Questions:

  • Should use_persis_return_sim be deprecated - if so now or next release. Requires ensuring all functions update to returning None (including in docs) - which should do anyway.
  • Should H0 be an argument to ensemble.run()?
@shuds13 shuds13 self-assigned this Jul 15, 2024
@shuds13 shuds13 changed the title Future release (1.4.1 or 1.5.0) Future release (1.4.1 or 1.5.0 or 2.0) Jul 15, 2024
@shuds13 shuds13 changed the title Future release (1.4.1 or 1.5.0 or 2.0) Future release (1.4.x or 1.5.0 or 2.0) Jul 27, 2024
@jlnav
Copy link
Member

jlnav commented Aug 20, 2024

Make sure persistent gens return None as first parameter unless want to return something.

Out of curiosity, I think functions that return "nothing" already have their output parameter captured as None, e.g.

>>> out = print("HELLO")
>>> type(out)
<class 'NoneType'>

Within the worker, current logic, probably can be dramatically improved:

            if out:
                if len(out) >= 3:  # Out, persis_info, calc_status
                    calc_status = out[2]
                    return out
                elif len(out) == 2:  # Out, persis_info OR Out, calc_status
                    if isinstance(out[1], (int, str)):  # got Out, calc_status
                        calc_status = out[1]
                        return out[0], Work["persis_info"], calc_status
                    return *out, calc_status  # got Out, persis_info
                else:
                    return out, Work["persis_info"], calc_status
            else:
                return None, Work["persis_info"], calc_status

Looks like if a gen returns None or otherwise, the intention may be captured anyway? I dunno

@shuds13
Copy link
Member Author

shuds13 commented Dec 12, 2024

Out of curiosity, I think functions that return "nothing" already have their output parameter captured as None, e.g.

If you look at most persistent gens in gen_funcs they return

return H_o, persis_info, FINISHED_PERSISTENT_GEN_TAG

not

return None, persis_info, FINISHED_PERSISTENT_GEN_TAG

which will result in the last set of points being added to the end of H as duplicates if we dont require use_persis_return_gen. So these should be replaced with None in most cases.

@jlnav
Copy link
Member

jlnav commented Dec 13, 2024

Fair enough; by the last set of points do you mean the last generated, but not sent out, set of points being added as duplicates? Would those points having sim_ids by that point prevent duplication? Maybe I'm missing something

@shuds13
Copy link
Member Author

shuds13 commented Dec 13, 2024

Fair enough; by the last set of points do you mean the last generated, but not sent out, set of points being added as duplicates? Would those points having sim_ids by that point prevent duplication? Maybe I'm missing something

Look at this code, which is typical for our persistent gens.

    while tag not in [STOP_TAG, PERSIS_STOP]:
        H_o = np.zeros(b, dtype=gen_specs["out"])
        H_o["x"] = persis_info["rand_stream"].uniform(lb, ub, (b, n))
        tag, Work, calc_in = ps.send_recv(H_o)


    return H_o, persis_info, FINISHED_PERSISTENT_GEN_TAG

H_o is sent to the manager in ps.send_recv(H_o). Then at end those points are returned (again). They get added twice - the manager will give new sim_ids. We decided to ignore the return H_o unless use_persis_return_gen is set, in order to prevent that. Also, bear in mind, users will have used these as templates to their own gens.

We should make our gens end with

return None, persis_info, FINISHED_PERSISTENT_GEN_TAG

unless we do something after the loop and actually make a different H_o.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Aspirational
Development

No branches or pull requests

2 participants