.github/workflows: Added ubi8-nightly.yml

[e10harvey]

• Includes various fixes in examples.

Summary:

• Adds a nightly run of examples in ubi8.
• Fixes some regressions and disables two tests (see example_SolarFieldOutput.solar_field_h_centroids failing image diff #51 and example_SolarFieldOutput.solar_field_h_centroids_names failing image diff #52).

[bbean23]

Feel free to merge. I do, however, have a couple of minor thoughts that I would like you to respond to, as you have time.

[bbean23]

Does this create a new container every night? If that's the desired behavior, that seems good to me, because it will help us to catch dependency errors faster. If this doesn't create a new container every night, then maybe it should. Thoughts?

[braden6521]

This reasoning makes sense to me. I don't know the implications to runtime/using resources of this though. Those would be my only concerns but I'm not well versed in this.

[e10harvey]

No, this runs pip install in the container image.

A image would be built with something like: docker build ... or podman build .... For example, see https://github.com/sandialabs/OpenCSP/blob/main/.github/workflows/docker.yml.

[bbean23]

That's more or less what I thought. Thanks!

Back to my original question, can we have a secondary container for the nightly tests that always updates to the latest packages? Really, I think the best set up would be:

• one Ubi8 container for PR unit tests & example nightly tests
• one Windows container for PR unit tests & example nightly tests
• one Ubi8 container for "pip --update" unit+example nightly tests
• one Windows container for "pip --update" unit+example nightly tests

All four containers should be created every time we merge to main. However, the second two containers should always be running with the latest version of all our dependencies, to try and catch dependency issues early.

What do you think @e10harvey? Is this a reasonable and achievable goal?

[e10harvey]

@bbean23: In general, I think this is a good idea. That being said, I think updating everything in the image every night is asking for trouble. If you would like to triage the nightlies when these fail, then I am OK with it. Otherwise, I think it would be best to setup a second nightly that updates everything in a separate image and runs examples + unit tests. Even with that setup, I am not volunteering for keeping this running : )

[bbean23]

Yep, a second nightly is exactly what I'm proposing. And I agree, you shouldn't be necessarily responsible for making it work. In fact, it would be helpful for me to learn how to set up a container like this. I'll make a ticket to do so.

[bbean23]

Why was this change made? My preference is to have setup code like this contained to within the main statement. Is that incompatible with unit tests somehow?

[bbean23]

If this is incompatible with unit tests, then can we do this instead?

def set_up():
    # Define output directory
    save_path = join(dirname(__file__), 'data/output/scene_reconstruction')
    ft.create_directories_if_necessary(save_path)

    # Set up logger
    lt.logger(join(save_path, 'log.txt'), lt.log.INFO)

    return save_path

if __name__ == "__main__":
    save_path = set_up()
    scene_reconstruction(save_path)

[bbean23]

Oh I see, it's because we're looking for methods that start with "example_". With that in mind, how do you feel about "example_main" instead of "example_driver"?

[e10harvey]

We can refactor to example_main, if you prefer.

[bbean23]

I have a slight preference for example_main. Probably not worth another pr just for that, though. 🤷‍♂️

[bbean23]

Feel free to delete this line. I did a similar thing in figure_management.py a while ago:

# matplotlib.use('tkagg') # BGB I don't think we should depend on this
mnger = plt.get_current_fig_manager()
if hasattr(mnger, 'window'):
    if hasattr(mnger.window, 'wm_geometry'):
        mnger.window.wm_geometry(f"+{ul_x}+{ul_y}")
    elif hasattr(mnger.window, 'geometry'):
        curr_dims = mnger.window.geometry().getRect()  # x, y, w, h
        mnger.window.setGeometry(curr_dims[0], curr_dims[1], ul_x, ul_y)

[braden6521]

All looks good to me. Thanks for fixing the example_... prefixes. I'll try to make sure examples are in that format in the future.