ACHMartin / seastar_project

4 stars 0 forks source link

Issue #264 #267

Closed DavidMcCann-NOC closed 1 year ago

DavidMcCann-NOC commented 1 year ago

Changes to address Issue #264 causing crashes of find_minima in the case of out-of-bounds x0.

In test cases with un-physical RSV and Sigma0 this fix does not always flag bad boundary conditions, however it does prevent the function from crashing during parallel computation.

ACHMartin commented 1 year ago

.

I assume you did some test. Any chance you could push them as unit_test? If too complicated to do it rapidly (half a day), I suggest you put your code for the test in a new issue and we will clean that up by then

DavidMcCann-NOC commented 1 year ago

.

I assume you did some test. Any chance you could push them as unit_test? If too complicated to do it rapidly (half a day), I suggest you put your code for the test in a new issue and we will clean that up by then

I'm not sure I understand - do you mean push my testing notebook for this issue but on the unit_test branch?

ACHMartin commented 1 year ago

.

I assume you did some test. Any chance you could push them as unit_test? If too complicated to do it rapidly (half a day), I suggest you put your code for the test in a new issue and we will clean that up by then

I'm not sure I understand - do you mean push my testing notebook for this issue but on the unit_test branch?

We have unit test in "tests/...". It will be good to add the tests you have done (quick test in a matter of seconds or less than 1 minute) in the unit_test, so we can test it systematically. If you don't have time to do it now, raise an issue with the link to your notebook

DavidMcCann-NOC commented 1 year ago

.

I assume you did some test. Any chance you could push them as unit_test? If too complicated to do it rapidly (half a day), I suggest you put your code for the test in a new issue and we will clean that up by then

I'm not sure I understand - do you mean push my testing notebook for this issue but on the unit_test branch?

We have unit test in "tests/...". It will be good to add the tests you have done (quick test in a matter of seconds or less than 1 minute) in the unit_test, so we can test it systematically. If you don't have time to do it now, raise an issue with the link to your notebook

This might take more time than that as there is an open unit_test issue (Issue #259) that potentially needs to be fixed first

ACHMartin commented 1 year ago

.

I assume you did some test. Any chance you could push them as unit_test? If too complicated to do it rapidly (half a day), I suggest you put your code for the test in a new issue and we will clean that up by then

I'm not sure I understand - do you mean push my testing notebook for this issue but on the unit_test branch?

We have unit test in "tests/...". It will be good to add the tests you have done (quick test in a matter of seconds or less than 1 minute) in the unit_test, so we can test it systematically. If you don't have time to do it now, raise an issue with the link to your notebook

This might take more time than that as there is an open unit_test issue (Issue #259) that potentially needs to be fixed first

I think this can be done despite of this. Pytest can fail on different items (unit test), it doesn't prevent to add a new unit test and running it (I think). Issue #259 is not very clear to me. Anyway, I think you don't have much time, but it will be a pity to lose the testing you have done. Keeping it in a new issue with the associated notebooks will certainly help to do the unit test for the try/catch in the future.