NREL / Wattile

Deep Learning-based Forecasting of Building Energy Consumption
BSD 3-Clause "New" or "Revised" License
9 stars 0 forks source link

Wattile trained model metadata should include Wattile version used to train #322

Closed stephen-frank closed 3 weeks ago

stephen-frank commented 1 month ago

...if it does not already

smithcommajoseph commented 1 month ago

How do we feel about this solution https://github.com/NREL/Wattile/pull/324?

smithcommajoseph commented 1 month ago

Continuing the conversation here, rather than the PR itself (just to keep things good and confusing 😅 )

I stand corrected. We could add the wattile version to the exported torch model. I don't love this from a 'separation of concerns' angle as it seems cleaner to keep the metadata separate from the model. However, we could add this relatively quickly and we wouldn't add a new file to the list of artifacts - which is a bonus.

What do you think @stephen-frank ?

stephen-frank commented 1 month ago

I am ok with any solution that allows SkySpark to easily read the version, either from a config file or by somehow instantiating the model and checking a property. It doesn't have to be in the torch model per se.

stephen-frank commented 1 month ago

Ah, missed this:

I'm wondering if we should we add a separate model_metadata or metadata file. It could minimally include the wattile_version, but perhaps we'll want to add more in the future - like specifics about the training ENV (some of which are captured by output.out). We could potentially even replace the output.out file with a machine readable metadata file.

What do y'all think?

Yes, something like that seems good.