Inf_Location error not tested

[davos-i]

Branch and PR to explore why Inf_Location can cause unexpected behaviour. Related to #108

Summary: there is an error in our code - but end_to_end testing is not set up to detect it. See below for details.

Checklist to be completed

Mark with [x] to show completed:

Fixes #108

• [x] No merge conflicts (if conflicts, merge main to branch and resolve before making PR)
• [x] Tests written (if feature added or refactored)
• [x] pytest passing
• [ ] Docstrings - minor
• [ ] Full documentation

[davos-i]

Firstly, the error is located here and I haven't fixed it yet because I wanted to figure out how it could go unnoticed by all of the testing. It was also hard to find because the variable names are generated by f strings, so not searchable in code.

https://github.com/CNM-University-of-Guelph/NASEM-Model-Python/blob/17159b9699b68e051310e0160e9225b4a3245df7/src/nasem_dairy/NASEM_equations/infusion_equations.py#L199

This + should be a *

[davos-i]

The testing issue: We should get in the habit of always writing our execute_model() out in full, as the list of default arguments is large and important. In the testing we have this:

https://github.com/CNM-University-of-Guelph/NASEM-Model-Python/blob/17159b9699b68e051310e0160e9225b4a3245df7/tests/integration/test_end_to_end.py#L78-L81

This is using default values for what should be tested. The full execute model looks like:

def execute_model(user_diet: pd.DataFrame,
                  animal_input: dict,
                  equation_selection: dict,
                  feed_library_df: pd.DataFrame,
                  coeff_dict: dict = constants.coeff_dict,
                  infusion_input: dict = constants.infusion_dict,
                  MP_NP_efficiency_input: dict = constants.MP_NP_efficiency_dict,
                  mPrt_coeff_list: list = constants.mPrt_coeff_list,
                  f_Imb: np.array = constants.f_Imb)

It's not safe to assume that our defaults shouldn't be tested, particularly because in infusion_dict we have a flag that is used to modify the way the code works.

In the first commit there are 2 JSON files, the one that I made by exporting from R and the one that is exported to the integration test dir by create_end_to_end_test_json.py. In this file there is the following code:

https://github.com/CNM-University-of-Guelph/NASEM-Model-Python/blob/17159b9699b68e051310e0160e9225b4a3245df7/dev_scripts/create_end_to_end_test_json.py#L166-L174

I don't think we should remove because we should be evaluating if our output matches the R code based on all values and not assuming that or constants (or flags) are correct. Instead I think we should pass to the testing function.

In our case, if we had given the Inf_Location from the R's JSON (which was 'Arterial') it would have set the Inf_Art value as 1 https://github.com/CNM-University-of-Guelph/NASEM-Model-Python/blob/17159b9699b68e051310e0160e9225b4a3245df7/src/nasem_dairy/NASEM_equations/infusion_equations.py#L170

and then it would have added 1 instead of multiple 1, resulting in incorrect InfArt_* values and the test would fail.

[BFieguth]

This set of integration tests is meant to confirm the Python and R implementations of NASEM 2021 are equivalent. We assume the constants are equal for these tests as the R version does not allow users to change these values (without modifying the code). If any constants are changed then the implementations are no longer equivalent and the current integration tests should fail. Perhaps we should have more labelling of the integration tests to clarify the purpose of tests.

Current integration tests are not set up to include an infusion input. Unit testing for the infusion module is incomplete. create_end_to_end_test_json.py should be refactored to include an infusion_input.

Once refactored new integration tests should be created to ensure infusion module is working properly. Unit testing of infusion module should also be completed.

[BFieguth]

The suggested changes have been made to the integration testing. The issue in calculate_infusion_data has been fixed. All tests are passing.

@davos-i I can't request your review since it's your PR. Can you review and check everything works on your system?

[davos-i]

Looks good on my end, thanks!