NASSP
NASSP copied to clipboard
orbiternassp
Update Specific Heat model
this update adds a model of specific heat that distinguishes between liquid and vapor phase. I was under the impression that this update would require a change to the internal energy value for every tank, but it seems to work just fine without it (temperatures are relatively accurate on initial load of mission scenarios). before this gets merged it should be tested against @n7275 FuelCellSystemsUpdate2 branch and @rcflyinghokie CMECS branch to ensure that it has desired effect.
In contradiction with my original comment on this pull request does require updating the internal energy of every hSystem. This pull request includes a commit, which updates this energy value in every scenario and the config files. For any additional user scenario, I have an update script https://gist.github.com/n7275/8cab7e348507ff4bce4749d09e2c3894 . It operates on all files ending in ".scn" in the directory in which the script is executed. Make a backup; there is no undo button. I intend on sharing this with the Forum, with the same cautionary notice, and it people don't have Python/want to do this themselves, I can of course do the upgrade for them.
I'm generally not a fan of introducing breaking changes unless absolutely required. Even then I'm very wary to push them through. Having a conversion script is definitely a step in the right direction. However I have the impression that NASSPs userbase is starting to grow and only a small portion of those are actively checking the work thread. Besides, most of them likely don't have the know-how or desire to setup Python and run that script. Nor is converting scenarios for individual users a scalable solution.
I've had an idea in the back of my head for a while, with this PR it might actually be time to implement it. Our scenarios already contain something like a version number, NASSPVER, which has been incremented each major release but isn't used otherwise. Some kind of check could be implemented that checks this variable to see if the scenario being used is compatible with the current NASSP version. If this is not the case some kind of message could be displayed to refer to some place (forum thread/wiki article/etc.) which documents the differences between scenario versions and (if possible) what changes are required to make the scenario compatible. This lessens the pain of making breaking changes to scenarios and prevents people simply giving up on NASSP because it is broken for them. Still these breaking changes should be made as little as possible but this way their atleast is some kind of notification system to prevent chaos and disappointment.
On Feb 1, 2022, at 10:33 AM, Thymo van Beers @.***> wrote: I'm generally not a fan of introducing breaking changes unless absolutely required. Even then I'm very wary to push them through. Having a conversion script is definitely a step in the right direction. However I have the impression that NASSPs userbase is starting to grow and only a small portion of those are actively checking the work thread. Besides, most of them likely don't have the know-how or desire to setup Python and run that script. Nor is converting scenarios for individual users a scalable solution.
I've had an idea in the back of my head for a while, with this PR it might actually be time to implement it. Our scenarios already contain something like a version number, NASSPVER, which has been incremented each major release but isn't used otherwise. Some kind of check could be implemented that checks this variable to see if the scenario being used is compatible with the current NASSP version. If this is not the case some kind of message could be displayed to refer to some place (forum thread/wiki article/etc.) which documents the differences between scenario versions and (if possible) what changes are required to make the scenario compatible. This lessens the pain of making breaking changes to scenarios and prevents people simply giving up on NASSP because it is broken for them. Still these breaking changes should be made as little as possible but this way their atleast is some kind of notification system to prevent chaos and disappointment.
I don’t have a scenario file in front of me but if NASSPVER appears early enough in the scenario file, it can set a global flag when seen during scenario load, and subsequent lines can be converted on the fly as required. This assumes the change is something that can be accommodated, like a new variable that can be safely defaulted or derived from other variables. Then you’d only have to tell the user about things that can’t be handled this way.
Yep, that crossed my mind too. It's the very first variable being written by NASSP. It's certainly an option but then you need to set clear boundaries to what extent you're going to support something. Defaults for simple variables that didn't yet exist would work as you can then use the default without incrementing NASSPVER. Defaults are generally prelaunch centric, so it would still require incrementing NASSPVER if that default doesn't work if the vessel is already in-flight. I wouldn't really want to go the way of maintaining some kind of scenario upgrade facility in code either. That could become high maintenance very quickly.
On Feb 1, 2022, at 10:33 AM, Thymo van Beers @.***> wrote: I'm generally not a fan of introducing breaking changes unless absolutely required. Even then I'm very wary to push them through. Having a conversion script is definitely a step in the right direction. However I have the impression that NASSPs userbase is starting to grow and only a small portion of those are actively checking the work thread. Besides, most of them likely don't have the know-how or desire to setup Python and run that script. Nor is converting scenarios for individual users a scalable solution. I've had an idea in the back of my head for a while, with this PR it might actually be time to implement it. Our scenarios already contain something like a version number, NASSPVER, which has been incremented each major release but isn't used otherwise. Some kind of check could be implemented that checks this variable to see if the scenario being used is compatible with the current NASSP version. If this is not the case some kind of message could be displayed to refer to some place (forum thread/wiki article/etc.) which documents the differences between scenario versions and (if possible) what changes are required to make the scenario compatible. This lessens the pain of making breaking changes to scenarios and prevents people simply giving up on NASSP because it is broken for them. Still these breaking changes should be made as little as possible but this way their atleast is some kind of notification system to prevent chaos and disappointment. I don’t have a scenario file in front of me but if NASSPVER appears early enough in the scenario file, it can set a global flag when seen during scenario load, and subsequent lines can be converted on the fly as required. This assumes the change is something that can be accommodated, like a new variable that can be safely defaulted or derived from other variables. Then you’d only have to tell the user about things that can’t be handled this way.
Is there any we could use the commit ID? I know a few other projects do this?
Breaking changes are a pain, and I can definitely see this could be extremely discouraging for someone, the scenarios of whom it breaks. I would imagine a non-insignificant portion of our userbase isn't even active on the forums, and even if they were, they'd need to be looking out for the right post in the right thread.
I'm sure this won't be the last one we do, but I also think the sooner we do necessary fundamental changes, the smaller impact in the long run. My intent with this update (and the vapor pressure/hVap update), was to make a change to the systemsSDK so that we would be happy with it for years to come, and would be less likely to run into fundamental impossibilities, or require even bigger breaking changes to correct down the road.
Before we merge this one, let's add some kind of "Your Scenario is from an Older Version, [something about if you have problems check in with us]".
On Feb 1, 2022, at 10:48 AM, Thymo van Beers @.***> wrote:
Yep, that crossed my mind too. It's the very first variable being written by NASSP. It's certainly an option but then you need to set clear boundaries to what extent you're going to support something. Defaults for simple variables that didn't yet exist would work as you can then use the default without incrementing NASSPVER. Defaults are generally prelaunch centric, so it would still require incrementing NASSPVER if that default doesn't work if the vessel is already in-flight. I wouldn't really want to go the way of maintaining some kind of scenario upgrade facility in code either. That could become high maintenance very quickly.
Personally, I’d only do it for one version back, maybe two, as a convenience for people following development so they don’t have to manually patch scenario files. People following packaged releases are unlikely to change releases mid-flight.
Is there any we could use the commit ID? I know a few other projects do this?
Not right now but that could be done in the future. You can add a compiler flag to define a macro with the id. NASSPVER has always been an integer though and currently supports x.y.z versioning. Not sure if we want to break from that.
Breaking changes are a pain, and I can definitely see this could be extremely discouraging for someone, the scenarios of whom it breaks. I would imagine a non-insignificant portion of our userbase isn't even active on the forums, and even if they were, they'd need to be looking out for the right post in the right thread.
I'm sure this won't be the last one we do, but I also think the sooner we do necessary fundamental changes, the smaller impact in the long run. My intent with this update (and the vapor pressure/hVap update), was to make a change to the systemsSDK so that we would be happy with it for years to come, and would be less likely to run into fundamental impossibilities, or require even bigger breaking changes to correct down the road.
Before we merge this one, let's add some kind of "Your Scenario is from an Older Version, [something about if you have problems check in with us]".
Oh I absolutely agree with you this update definitely needs to be included, breaking change or not. I'll try to whip something up. At the very minimum a notification, but porting from the last to new version should be doable.
I've added a very basic check that compares the scenario version to the compiled constant and shows a message on a mismatch. You can find it in the scn_check branch.
We could do anything from here really.
