Add financial distress#198
Conversation
b5a380c to
f28f96b
Compare
andrewbaxter439
left a comment
There was a problem hiding this comment.
This code seems good! Failing validation tests - is that likely caused by different initial population?
f28f96b to
5ff9d91
Compare
I thought so at first, but I tried to keep the initial population unchanged except for the financial distress variable, and there are still small differences. (In theory, there shouldn't be, because the new process doesn't interact with any existing processes) I think because I've added a process that involves additional random draws, all the subsequent random draws will be different, leading to slightly different random noise in the output. I have to say that diminishes the value of that test a bit. But maybe we can be content with verifying that the output statistics remain approximately the same? |
|
I had wondered if the additional draws were partly to blame, though these don't seem to affect the previous draws in the same class InnovationsTest {
@Test
public void checkSame() {
Innovations innovation1 = new Innovations(2, 100);
Innovations innovation2 = new Innovations(3, 100);
assertTrue(innovation1.getDoubleDraw(0) == innovation2.getDoubleDraw(0) );
assertFalse(innovation1.getDoubleDraw(1) == innovation2.getDoubleDraw(2) );
}
}... passes fine? |
|
...something to discuss with @van De Ven, Justin W ***@***.***>
perhaps?
…On Fri, 13 Jun 2025 at 09:47, Andrew Baxter ***@***.***> wrote:
*andrewbaxter439* left a comment (simpaths/SimPaths#198)
<#198 (comment)>
I had wondered if the additional draws were partly to blame, though these
don't seem to affect the previous draws in the same Innovations object:
class InnovationsTest {
@test
public void checkSame() {
Innovations innovation1 = new Innovations(2, 100);
Innovations innovation2 = new Innovations(3, 100);
assertTrue(innovation1.getDoubleDraw(0) == innovation2.getDoubleDraw(0) );
assertFalse(innovation1.getDoubleDraw(1) == innovation2.getDoubleDraw(2) );
}
}
... passes fine?
—
Reply to this email directly, view it on GitHub
<#198 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACOK4OAFPD7UVKW6JSKQDHL3DKFZ3AVCNFSM6AAAAAB65GG3VWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSNRZGU3TKMRVGI>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
07ef062 to
7409aab
Compare
|
Awesome that everything's working now! Shouldn't add any breaking changes so if this were able to be merged (@pbronka /@justin-ven) I'd be keen to bring this new parameter into re-estimating mental health effects? |
869acec to
1d8780f
Compare
1d8780f to
77816cc
Compare
|
@andrewbaxter439 I've updated this based on discussions:
|
|
Awesome that this is working - think ready to merge I'd say! |
What
Why
Resolves #181.
To do before merging
Model specification for financial distress process
Logit model estimated in UKHLS. Predictors: