Closed yoelcortes closed 4 years ago
Hi Yoel,
Thank you for the comprehensive feedback. I have incorporated it into the file, hopefully making it quite a bit better. The changes are:
1) The design of the database is now - load all the small databases; load the elements. If a search fails, load the large database; load the small databases; load the elements. Search again. This makes sense to me because the small databases are, well, small. 2) We seem to have different experiences with name conflicts. For me, my intention has always been for the small databases to supersede the values in the big database I can't really edit. If you have a conflict, please let me know. 3) I renamed autoload_next to finished_loading. 4) The element search is necessary to handle searches like "nitrogen" and 1 correctly. I cleaned it up and added some comments. 5) I implemented a fast cache size limit and did the wrapper function like you suggested - a clear improvement.
Thank you for the feedback and I hope you can find the issue you had with ions so we can fix it.
Sincerely, Caleb
Thanks for working on this and asking for my comments. Here are my comments at first glace:
search_chemical
function searches through the periodic table first... lines 361-379 could be deleted and the performance would be better.main_db
,user_dbs
,can_autoload
, andloaded_main_db
property/attributes can probably be simplified. I would suggest having two lists, aloaded_files
and anunloaded_files
. We could pop unloaded_files, load them, and append them to loaded_files and keep autoloading until we find the chemical or run out of unloaded files. This would also make it more clear which files have been loaded (in contrast to acan_autoload
property), and the possibility of staging files to autoload (which is not possible right now inchemicals
; but is possible in thermosteam).Here is one additional small change:
I hope these comments are helpful and I didn't misinterpret any code, Thanks!