search

pyswmm / Stormwater-Management-Model

PySWMM Stormwater Management Model repository
Other
99 stars 77 forks source link

Update .rpt text to indicate OWA SWMM codebase #365

Closed jennwuu closed 2 years ago

jennwuu commented 2 years ago

Related to discussion from #352

It is important user understands the source of model results [re-replicating results, documentation, etc]. When a user views .rpt, the header is not descriptive to indicate which source of engine used. I think this is important as we add more features in the api.

At the moment, the .rpt file shows:

  EPA STORM WATER MANAGEMENT MODEL - VERSION 5.1 (Build 5.1.14)
  --------------------------------------------------------------
jennwuu commented 2 years ago

We should close this issue in the next release since #352 is closed.

Can we decide on the header text for the .rpt file?

@bemcdonnell @michaeltryby @cbuahin @abhiramm7

bemcdonnell commented 2 years ago

@jennwuu and @michaeltryby, How about this?

  EPA STORM WATER MANAGEMENT MODEL  (OWA-Build 5.1.14)
  Build: asdfn92hf98haf9a8hf43
  --------------------------------------------------------------
michaeltryby commented 2 years ago

This is what is in there now. It seems fine. 

report_writeLogo()
...
OWA STORM WATER MANAGEMENT MODEL - VERSION v%s (Build %.10s)"

I don't think we should add another line to the header as that may throw off someone parsing the report.

bemcdonnell commented 2 years ago

@michaeltryby I agree. adding the SHA is probably redundant.

jennwuu commented 2 years ago

@michaeltryby PCSWMM parses text between brackets to relay build information from .rpt. Can we add OWA in the bracket to something like this?

OWA STORM WATER MANAGEMENT MODEL - VERSION v%s (OWA Build %.10s)
bemcdonnell commented 2 years ago

@michaeltryby i think we are converging

...
OWA STORM WATER MANAGEMENT MODEL - VERSION v%s (OWA Build v%s.%.10s)"
michaeltryby commented 2 years ago

We are moving to semantic version standard - major, minor, patch. Previously, patch was being referred to as build but technically that's not accurate. In the version header I created BUILD_ID is a time stamp.

bemcdonnell commented 2 years ago

so @michaeltryby, for example....

OWA STORM WATER MANAGEMENT MODEL - VERSION (OWA 6.1.0)
michaeltryby commented 2 years ago

Why does OWA need to be in there twice? In case they don't see it the first time?

jennwuu commented 2 years ago

@michaeltryby PCSWMM parses text between brackets to relay build information from .rpt. Can we include additional OWA text in the bracket?

bemcdonnell commented 2 years ago

@jennwuu,

Let's go with your choice for now:

OWA STORM WATER MANAGEMENT MODEL - VERSION 5.1 (OWA 5.1.14)
michaeltryby commented 2 years ago

We have new variables in the swmm.dll that encode more version and build information than ever before. We should get away from parsing the report file for information and create and use APIs.

bemcdonnell commented 2 years ago

@michaeltryby, we can circle back before the next release.

abhiramm7 commented 2 years ago

This issue is resolved in https://github.com/OpenWaterAnalytics/Stormwater-Management-Model/pull/372