[WIP] clean 'newborn' code on IndShockConsumerType.getShocks(), fixes #494

[sbenthall]

This PR cleans the code in IndShockConsumerType.getShocks():

• Less duplicate code
• More efficient runtime--doesn't compute 'newborn' shocks twice

[llorracc]

@sbenthall, am glad to see you doing the cleanup work of this kind.

But before you do much more of it, I think we need to confront an issue that we have talked about many times but not reached any answer to: We need to figure out a way to devise some tests for whether the SUBSTANTIVE outputs of anything change when there has been a code change. Travis will tell us whether any code 'breaks' in the sense of not executing, but it is likely that at some point someone will make what they think is an innocuous "cleanup" push which will pass Travis but will meaningfully (and wrongly) change quantitative results.

We should start with some one particular example, a REMARK. If necessary, we can modify it to construct all of its output in some simple machine-readable way that excludes meaningless stuff like creation timestamps etc. Then the test would be basically whether the new output files generated by rerunning it (say, the do_all.sh script) are different at all from the output files generated on the last run. If so, we would flag it as an error.

I'm about to do a bit of work on BufferStockTheory, and will see if that is the REMARK we should use to start with.

[sbenthall]

@llorracc , I think this opens up a lot of questions around release management and testing that are way beyond the scope of this PR, which is for a trivial change.

I've made a new issue for the broader issue you've raised:
#504

[sbenthall]

t_age is different from t_cycle, and the change in the original commit breaks things.

t_age always increments, while t_cycle cycles.

[sbenthall]

See also #327

[sbenthall]

See also #326

[sbenthall]

Good news: the automated test designed to catch if this PR would create a breaking change has caught that this PR would create a breaking change. The system works.

On the point of design, I hope I can get clarity on a point from @mnwhite 👍

If I understand correctly, a newborn agent must always be born at the 0th cyclic period.

In other words, all newborn agents are born in January. It's impossible to be born in August.

Is that: (a) correct? or (b) intentional?

This is related to #562 and #563.

[mnwhite]

I never saw this PR until now. Yes, a newborn is always born at cycle 0, so that the concept of the cycle is idiosyncratic rather than aggregate. My intention was that a user could simply write a subclass with an alternate simBirth method if they wanted an aggregate cycle.

…

On Wed, Mar 11, 2020 at 12:29 PM Sebastian Benthall < ***@***.***> wrote: Good news: the automated test designed to catch if this PR would create a breaking change has caught that this PR would create a breaking change. The system works. On the point of design, I hope I can get clarity on a point from @mnwhite <https://github.com/mnwhite> 👍 If I understand correctly, a newborn agent must always be born at the 0th cyclic period. In other words, all newborn agents are born in January. It's impossible to be born in August. Is that: (a) correct? or (b) intentional? This is related to #562 <#562> and #563 <#563>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#499 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADKRAFLF63GSNARVAATMELLRG636XANCNFSM4KR6CM7Q> .