-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add setter to fireID property in FireObj.Fire #169
base: conus-dps
Are you sure you want to change the base?
Conversation
@@ -494,6 +499,10 @@ def isignition(self): | |||
def fireID(self): | |||
return self._fid | |||
|
|||
@fireID.setter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw this is the only actual change. All the other stuff was style syntax my IDE made. sorry!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that's fine - comes from a lot of this being written in jupyterhub without linting enabled.
@@ -494,6 +499,10 @@ def isignition(self): | |||
def fireID(self): | |||
return self._fid | |||
|
|||
@fireID.setter | |||
def fireID(self, newid): | |||
self._fid = newid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really enough? Don't we need to explicitly update the backing objects to change the fid from the old one to the new one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh possibly... I can dig into this more. Are there other good fires to look at that go from Dec to Jan? Out of the last 10 years, I've only had one active fire in CA (Thomas 2017) that did that and it was barely active. Possibly some in the southern hemisphere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am certainly not the person to ask :) maybe ask around on slack or tag people here to pull them into the conversation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Lisa! Thanks for digging into this. I suspect we can help you find more examples of cross year fires, and actually have put this on the roadmap of things we need to deal with as we scale to global runs. Just so you know, the GSFC team is at AGU this week, so you probably won’t hear much from us in the next few days. However, we do have some southern hemisphere results and I’d be happy to generate more after the conference with an eye towards that Dec 31/Jan 1 crossover. Let’s touch base next week if needed.
Fire IDs reset at the start of a calendar year--see
FireObj.Allfires.newyear_reset()
.FireID
is a property that needs to be updated, but needed a setter to be able to do so. Added that here. This only matters for fires that are actively growing into the January of the following calendar year. Ex/ 2017 Thomas fire in CA.I tested this out in
notebooks/xx_constrainbyshape.ipynb
.