Fix bad units in IDD for swimming pool misc power input#4637
Fix bad units in IDD for swimming pool misc power input#4637Myoldmopar merged 1 commit intodevelopfrom
Conversation
Fix bad units in IDD for swimming pool misc power input
| @Myoldmopar Thanks. So, should I post a new issue to suggest that we add an IDD validator to the CI tests? Or a pivotal ticket? IDF Editor has code in it, but I think there are some other scripts out there already (perhaps the one that @mmAtTs used for the curve referencing cleanup) that could be the basis for checking that all of the \codes are valid, and all of the \object-list referencing is valid. |
| I'd say make a Pivotal ticket. I have an idd parser that would be very easy to use here, but it is in VB.Net, so it would be WIndows only...but that's all we need anyway. At one point, I had started translating it to python, so it might be a very light workload to get this validation working well. Feel free to assign it to me. |
| Let me just say I am not in favor of VB or Python validation being bolted on here. The idea of the project is bring your compiler and cmake and press the compile button. Those should be the dependencies. The alternative starts to look like a Frankenstein of Legos, Erector Set, and Lincoln Logs. It is bad enough that Python is required to do regression tests. Sure you can say, but this is an isolated, advanced use case. I don't agree. This use case of idd validation seems especially touchy, because those are facilities that are in one way or another captured in the EnergyPlus code itself. Don't compete with that or reinvent it. If you want to use idd validation outside of the engine, then factor that component out of EnergyPlus and use that. I know it is more work, but that is exactly the kind of modular (and unit testable) EnergyPlus code that you want anyway. See my comments to @refaqtor (I don't know how to link to specific comments) about reusing idf parsing that already exists. I think the same thing applies here. #4636 |
| (@kbenne, if you look at the top of the comment itself, there is a time-note, such as "Commented 2 minutes ago", the time text is the link to the specific comment) |
| Very nice. #4636 (comment) |
| @kbenne I understand the concern of adding more dependencies. The reason I suggested this is because EnergyPlus does not validate or even use many of the \codes. Some of them it does, but many it does not, and for some it's more forgiving. But IDF Editor and presumably other tools do use them and complain when they are not valid. I wasn't intending this to even be part of the build, but rather a utility that runs on the CI just like mathdiff (assuming we're still using that for the regressions - or equivalent). Or we could add more validation in to EnergyPlus, but that may slow down the IDD parsing. I'll open a pivotal ticket and we can discuss further under that. |
@Myoldmopar @kbenne Would be good to get this in for the next iteration build.