search

ACEsuit / mace

MACE - Fast and accurate machine learning interatomic potentials with higher order equivariant message passing.
Other
413 stars 157 forks source link

bug in print_git_commit() #400

Closed venkatkapil24 closed 2 months ago

venkatkapil24 commented 2 months ago

The way the print_git_commit function in mace/build/lib/mace/tools/scripts_utils.py is defined it looks for a .git/ in the directory where the script is run. I think this is unexpected behaviour.

There is also a logical bug in the way it is implemented. If there is an issue with fetching the commit, the code doesn't break. It instead prints the exception as a warning and returns a None type to encode. This leads to an error at the stage of returning the trained model as a none type doesn't have an encode method. So either the code should stop at the beginning or it should return a null string "" or similar (with an encode method).

@ilyes319 if you are okay with it, I am happy to open a PR to develop with the a return "" instead on return None in mace/tools/scripts_utils.py#L133

ilyes319 commented 2 months ago

If you pull the latest main, this should be already fixed: https://github.com/ACEsuit/mace/blob/90f49391847d86d422fa20dd02d6c14136d68552/mace/cli/run_train.py#L793

venkatkapil24 commented 2 months ago

okay, I didn't realize we could do fine-tuning on the main branch. It may be worth keeping the develop branch in sync with main.

ilyes319 commented 2 months ago

it should also be in the develop.