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

Make all truncations/max_order/number parameters key/value pairs in dictionary #15

Closed
ngraymon opened this issue Jul 9, 2021 · 13 comments

Comments

@ngraymon
Copy link
Owner

ngraymon commented Jul 9, 2021

image
image

Rather then having them be a list of numbers orindividual keywords (which makes the function definitions super crazy long and unreadable) lets just have all of these 'master/parent' functions take an argument truncation which should always be a dictionary. Also add in simple checks for each function to make sure the expected keys are present and that the numbers provided are in an acceptable range (0 < N < max_number is probably fine for all of them, define the maximums at the top of the file)

Also we probably want the dictionary keys to be expanded in full so as to not confuse them with the variables used in the functions.
Meaning

maximum_h_rank becomes maximum_hamiltonian_rank

maximum_cc_rank becomes maximum_coupled_cluster_rank

so like just below here

image

truncation_maximums = {
    maximum_h_rank: 6,
    maximum_cc_rank: 6,
    s_taylor_max_order: 6,
    omega_max_order: 6,
    max_residual_order: 8, 
    max_w_order: 8,
    dt_order: 8,
}

and then around line 3265 (cut out the red lines)
image

for key in ["maximum_h_rank", "maximum_cc_rank", "s_taylor_max_order", "omega_max_order"]:
   assert key in truncations, f"Missing key {key = :s} in provided dictionary {truncations=:}"
   assert truncations[key] <= truncation_maximums[key], f"Key {key = :s} is over the maximum of {truncation_maximums[key] = :}"
maximum_h_rank = truncations["maximum_h_rank"]
maximum_cc_rank = truncations["maximum_cc_rank "]
s_taylor_max_order = truncations["s_taylor_max_order "]
omega_max_order = truncations["omega_max_order "]
@ngraymon
Copy link
Owner Author

ngraymon commented Jul 9, 2021

reminder that my code samples don't have the long form names for the keys, but they should

@ngraymon
Copy link
Owner Author

changed due date to March 24, 2022

@ngraymon
Copy link
Owner Author

So we should have a python file called truncations.py which defines namedtuple's for the different truncation types.

For example:

this truncation is valid for the z_t ansatz (see https://git.uwaterloo.ca/ngraymon/exploratory-project/-/blob/implement_pytest/latex_zhz.py#L1723)

the s_taylor_max_order positional argument is 'unwrapped' from truncations but never actually used

maximum_h_rank = 2
maximum_cc_rank = 6
omega_max_order = 3

this truncation is valid for the full cc ansatz

maximum_h_rank = 2
maximum_cc_rank = 6
s_taylor_max_order = 2
omega_max_order = 3

but for the eT_z_t ansats we need a truncation like this

maximum_h_rank = 2
maximum_cc_rank = 4
maximum_T_rank = 1
eT_taylor_max_order = 4
omega_max_order = 4

@ngraymon
Copy link
Owner Author

The file should have similar verification functions and (some sort of) creation functions as in the deep tests _verify_lhs_keys and so forth _basic_lhs_consistency

@ngraymon
Copy link
Owner Author

Per 46c0c555624deede3ccd32ad05ac4853c3c196d0 one way to do this is using enums
I've attached an example that I've used before

vibronic_model_keys.py

also note that enum docs are here : https://docs.python.org/3.8/library/enum.html?highlight=namedtuples

and then you can import it like so

from .vibronic_model_keys import VibronicModelKeys as VMK
print(VMK.E)

@ngraymon
Copy link
Owner Author

Saving to json looks like:

def _save_to_JSON(path, dictionary):
    dict_copy = copy.deepcopy(dictionary)
    VMK.change_dictionary_keys_from_enum_members_to_strings(dict_copy)
    """ converts each numpy array to a list so that json can serialize them properly"""
    for key, value in list(dict_copy.items()):
        if isinstance(value, (np.ndarray, np.generic)):
            if np.count_nonzero(value) > 0:
                dict_copy[key] = value.tolist()
            else:
                del dict_copy[key]
        else:
            log.debug(f"Value {value} with Key {key} does not appear to be an ndarray")

    with open(path, mode='w', encoding='UTF8') as target_file:
        target_file.write(json.dumps(dict_copy))

    return

def save_model_to_JSON(path, dictionary):
    """ wrapper for _save_to_JSON
    calls verify_model_parameters() before calling _save_to_JSON()
    """
    verify_model_parameters(dictionary)
    log.debug(f"Saving model to {path:s}")
    _save_to_JSON(path, dictionary)
    return

and loading

def _load_from_JSON(path):
    """returns a dictionary filled with the values stored in the .json file located at path"""

    with open(path, mode='r', encoding='UTF8') as file:
        input_dictionary = json.loads(file.read())

    VMK.change_dictionary_keys_from_strings_to_enum_members(input_dictionary)

    for key, value in input_dictionary.items():
        if isinstance(value, list):
            # if we don't predefine the shape, can we run into problems?
            input_dictionary[key] = np.array(value, dtype=F64)

    # special case to always create an array of energies that are 0.0 if not provided in the .json file
    if VMK.E not in input_dictionary:
        A, N = _extract_dimensions_from_dictionary(input_dictionary)
        shape = model_shape_dict(A, N)
        input_dictionary[VMK.E] = np.zeros(shape[VMK.E], dtype=F64)

    # TODO - design decision about which arrays to fill with zeros by default?

    return input_dictionary
def load_model_from_JSON(path, dictionary=None):
    """
    if kwargs is not provided then returns a dictionary filled with the values stored in the .json file located at path

    if kwargs is provided then all values are overwritten (in place) with the values stored in the .json file located at path
    """
    log.debug(f"Loading model from {path:s}")

    # no arrays were provided so return newly created arrays after filling them with the appropriate values
    if not bool(dictionary):
        new_model_dict = _load_from_JSON(path)

        # TODO - we might want to make sure that none of the values in the dictionary have all zero values or are None

        verify_model_parameters(new_model_dict)
        return new_model_dict

    # arrays were provided so fill them with the appropriate values
    else:
        verify_model_parameters(dictionary)
        _load_inplace_from_JSON(path, dictionary)
        # check twice? might as well be cautious for the moment until test cases are written
        verify_model_parameters(dictionary)

    return

@ngraymon
Copy link
Owner Author

  1. Make to sure to have a wrapper function for the load which checks the max values of the dictionary and for the presence of all the necessary keys
def load_trunc_from_JSON(path, dictionary):
    # steal from vibronic
  1. add check for max values in the _verify_*_truncations() functions

  2. expand save and load functions for each of the different truncation 'groups' so like
    save_*_trunc_to_JSON becomes save_fcc_trunc_to_JSON and the same for load_*_trunc_from_JSON

  3. something like this?

def save_to_JSON(path, dictionary):
    """ to make cleaner code this ATTEMPTS to automatically determine dictionary type
    and then save the appropriate truncation 
    """
    key_list = list(dictionary.keys())

    # assume fcc?
    if len(key_list) == 4:
        for key in TruncationsKeys.fcc_key_list()
            if key not in key_list
                raise Exception("something")

        # only get here if all keys are in key_list
        else:
            _save_fcc_trunc_to_JSON(path, dictionary)

    elif len(key_list) == 5:
        for key in TruncationsKeys.eT_z_t_key_list()
            if key not in key_list
                raise Exception("something")

        # only get here if all keys are in key_list
        else:
            _save_fcc_trunc_to_JSON(path, dictionary)

@ngraymon
Copy link
Owner Author

Then the next step would be to start replacing relevant parts inside _glue_.py
so

from truncations import TruncationsKeys as tkeys

fcc_trunc = {
    tkeys.H: 2,
    tkeys.H: 6,
    tkeys.H: 2,
    tkeys.H: 3,
}

and instead of

assert len(truncations) == 4, "truncations argument needs to be tuple of four integers!!"

we want to call some verification function from truncations.py

and also extract out like

maximum_h_rank = truncations[tkeys.H]
maximum_cc_rank = truncations[tkeys.CC]
s_taylor_max_order = truncations[tkeys.S]
omega_max_order = truncations[tkeys.P]

@ngraymon
Copy link
Owner Author

    tkeys._verify_eT_z_t_truncations(truncations)
    maximum_h_rank = truncations[tkeys.O]
    maximum_cc_rank = truncations[tkeys.O]
    maximum_T_rank = truncations[tkeys.O]
    eT_taylor_max_order = truncations[tkeys.O]
    omega_max_order = truncations[tkeys.O]

@ngraymon
Copy link
Owner Author

to replace the assert len(truncations) == 5 etc buisness

@ngraymon
Copy link
Owner Author

On the 29th some comments:

  • for me the priority is the file based method as the default approach
  • in latex_full_cc.py extend the remove_f_terms flag into the generate_full_cc_latex function so that from _glue_.py you can pass in the appropriate dictionary key/pair value associated with f terms
  • for all the top level generation functions, switch them to be of the form def func(truncations, **kwargs):
    and then unpack the kwargs similar to the truncations.
# if empty dict
if not bool(kwargs):
    kwargs = default_kwargs
else:
    # maybe do value checks?
    for key, value in default_kwargs.items():
        if key in kwargs:
            assert type(value) == type(kwargs[key])
    default_kwargs.update(kwargs)
  • don't focus on the file load/save aspect of only_ground_state and f term business at the moment.
  • expand out the _make_trunc function, eventually this should move into the truncations.py file but for now lets just make it work and move on to other things. Just do simple length checks like we previously used to appropriately sort it into the different ansatz`
  • fix commit ad38a65611de918735a717d25a7b19810cd88241 by removing it on a new branch
  • get rid of cd89992c718e31e107640c84c01952e56c8cedf8 and the other one which are just redudandnt

@ngraymon
Copy link
Owner Author

The last thing is after doing all of the above change the blanket test(as per #43) to pass and match the new specs

@ngraymon
Copy link
Owner Author

Then we can close this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant