Migrate combustion and EPW-reader from Hive.Core to Hive.IO

maxenceryan commented 2 years ago

Issues

Closes #686

Description

Migrate the epw-reader from ghpy to Hive.IO

EPW-reader now reads the EPW class given by the Hive.Environment instead of re-parsing the EPW file
ins and outs are the same except
- formatting of City, Country is slightly different
only main template and a new environment GH test file were changed

TODO:

@christophwaibel in many gh example files, a simple path is given to the python EPW reader, which now no longer works. Should I change all the testfiles to reflect the proposed set up (epw-path -> environment -> hive inputs -> env distributor -> epw reader instead of just epw-path -> epw-reader)? Or is it better to keep an option for simply giving the path? We could also move the parsing to the epw-reader instead of the environment component.

Checklist

[ ] Remove debugging artifacts (if needed)
[x] Rebuild the Setup_Hive.exe
[ ] Update Hive Wiki (if needed)
[ ] Update InformationalVersion attribute in Hive.IO/Properties/AssemblyInfo.cs (if it's a new release)
[x] Update most important Grasshopper template(s); as of now /GrasshopperExamples/LectureExercises/EaCS3_E04_Hive_Template.gh

christophwaibel commented 2 years ago

hey @maxenceryan , I suggest to keep an option for a simple workflow without needing to go through the whole environment->envDistributor pipeline. Yes, I think it makes sense to move the parsing to the EpwReader component. Please also note our naming convention of using a Prefix Gh(...) for all grasshopper components

maxenceryan commented 2 years ago

an issue with having parsing only in the EPW distributor is that the GhEnvironment component actually needs EPW already parsed in order to set a couple of the default energy potentials, which are passed along at the EnvDistributor.

l69-71 in Environment.SetDefaultEnergyPotentials()

            this.EnergyPotentials[6] = new Air(this.Horizon, null, null, null, this.EpwData.DryBulbTemperature);
            this.EnergyPotentials[6].Name = "DryBulbTemperature";
            this.EnergyPotentials[7] = new Radiation(this.Horizon, this.EpwData.GHI);

We couuuld just say either plug in an Hive.IO.EPW class object OR the filepath into the EPW Distributor component, with priority given to the EPW object if not null.

christophwaibel commented 2 years ago

an issue with having parsing only in the EPW distributor is that the GhEnvironment component actually needs EPW already parsed in order to set a couple of the default energy potentials, which are passed along at the EnvDistributor.

l69-71 in Environment.SetDefaultEnergyPotentials()
            this.EnergyPotentials[6] = new Air(this.Horizon, null, null, null, this.EpwData.DryBulbTemperature);
            this.EnergyPotentials[6].Name = "DryBulbTemperature";
            this.EnergyPotentials[7] = new Radiation(this.Horizon, this.EpwData.GHI);
We couuuld just say either plug in an Hive.IO.EPW class object OR the filepath into the EPW Distributor component, with priority given to the EPW object if not null.

right. then let the GhEnvironment already parse the epw and, as you said, the GhDistEpw either reads in a path (and parses the epw itself), or reads in an epw object and doesn't need to parse itself anymore.

the boolean, I might have explained it incorrectly. So the parsing should happen either way, but just dumping out the DataLists should be optional

maxenceryan commented 2 years ago

sounds good, your comment and commit make sense to me. Hopefully I can wrap this up by monday

maxenceryan commented 2 years ago

@christophwaibel ok so I finished up the EPW reader stuff as per the comments. You can see how it compares to the python solution in GrasshopperExamples\Testing\Hive_Environment_Test.gh. The assert code for comparing C# and python component is a tad messy... but it would throw an error if they didnt match.

You can also look in GrasshopperExamples\Testing\Hive_Distributor_Test.gh to see how it would look like from now on.

A few things;

I still need to update the EPW component in the other GH files, so a commit will be incoming.
Once you are done with your review, it'd be nice if you could:
- comment out the python epw-reader building in build.cmd
- delete the python component and assert madness in GrasshopperExamples\Testing\Hive_Environment_Test.gh.
Finally, i had already migrated the simple combustion components, so they are still there. As you said, we don't need them anymore, so should I for sure delete them or save them somewhere?

Thanks!

christophwaibel commented 2 years ago

A few things;

I still need to update the EPW component in the other GH files, so a commit will be incoming.

should I wait for another commit? otherwise, you can merge this PR, @maxenceryan

comment out the python epw-reader building in build.cmd

can't do yet, because there is a ghpython component in the main template that relies on following lines from epw_reader.py:

clr.AddReferenceToFileAndPath(os.path.join(path, "Libraries\Hive", "Hive.IO.gha"))
import Hive.IO.EnergySystems as ensys

I'll make a separate issue about it.

delete the python component and assert madness in GrasshopperExamples\Testing\Hive_Environment_Test.gh.

done

Finally, i had already migrated the simple combustion components, so they are still there. As you said, we don't need them anymore, so should I for sure delete them or save them somewhere?

as discussed before in the sprint: let's keep them for now

ChrisZenhub commented 2 years ago

@maxenceryan when merging with the current master, you can keep the EaCS3_E04_Hive_Template.gh from this PR and overwrite master. But @mmatache25 did some changes to the EnergySystems Hizard, so keep an eye on what you merge from Hive.IO.csproj.

Also, I'm not sure if this PR already merged the updated PV / BIPV catalogues

maxenceryan commented 2 years ago

ok so merging now. Last thing I did:

removed Hive.Core.combustion from build.cmd as it is now in Hive.IO. NSI script also does not install it and removes the ghpy if it was there from a previous version
kept epw-reader in build.cmd and as part of installer for now
fixed the compiling script for BIPV, it is now compiled to JSON with PV and ST (although will not be accessible for the user until #677)

architecture-building-systems / hive