check that equations_of_motion are not AEs

[Peter230655]

Check whether the equations of motions contain differential equation, and rais a ValueError if they are AEs.
I used @tjstienstra idea, which seems to be VERY fast!
Should take care of issue # 228

[moorepants]

You'll need to check this on something at least the size of the bicycle model to make sure it doesn't slow down things. I'm generally fine having a "garbage in, garbage out" design, especially when there can be significant performance costs for checking inputs.

[Peter230655]

You'll need to check this on something at least the size of the bicycle model to make sure it doesn't slow down things. I'm generally fine having a "garbage in, garbage out" design, especially when there can be significant performance costs for checking inputs.

The bicycle model does not run with me, and I still could not lokate the error.
I tested this two liner:

zeit = time.time()
if eom.has(sm.Derivative) == False:
    raise ValueError(.....)
duration = time.time() - zeit

with the eom having 23,000 operations, and duration was less than 10^(-15).
Is this testing of duration inadequate?
Should my eom have many more operations?
(Of course my eom had some sm.Derivative very 'soon', and I believe has(...) turns true as soon as it has found the first one.)

[moorepants]

(Of course my eom had some sm.Derivative very 'soon', and I believe has(...) turns true as soon as it has found the first one.)

If that is true, then it should be pretty fast.

[moorepants]

Suggested change

	raise ValueError('The equations of motion must be DEs or DAEs, not AEs.')
	raise ValueError('No time derivatives are present, the equations of motion must be ordinary differential equations (ODEs) or differential algebraic equations (DAEs),')

[moorepants]

This needs a unit test to make sure the error raises.

This will isn't completely robust in the sense that it may find a Derivative that is not a time derivative. In the case a user passes in equations with derivatives of variables other than time, the error message is misleading.

[Peter230655]

(Of course my eom had some sm.Derivative very 'soon', and I believe has(...) turns true as soon as it has found the first one.)

If that is true, then it should be pretty fast.

As per chatGPT it is true, that it stops as soon as it has found the first term...

[moorepants]

I'm not the type that listens to anything ChatGPT may say...

[Peter230655]

This needs a unit test to make sure the error raises.

This will isn't completely robust in the sense that it may find a Derivative that is not a time derivative. In the case a user passes in equations with derivatives of variables other than time, the error message is misleading.

I tested it with a small example, where I simply deleted the .diff(t) in all the terms. This worked, it raised the error.
Seems, I cannot find sm.Derivative(t) this way, I guess it is not an atom. I guess, I could find sm.Derivative(t similar to how I do it in plot_constraint_violations. But likely this is slow.
So, better drop this PR?

What I tried earlier was this:

• converted the eom to a string
• then I tested 'Derivative' in str(eom)

With 23,000 operations, this took about o.5 sec.
Like this, I can look for what I want, but time consuming.
Worth doing it?
I think, too complicated, I checked.
I guess, either we can accept the ambiguity or better drop it: garbage in / garbage out!

[Peter230655]

I'm not the type that listens to anything ChatGPT may say...

likely smart - but it has helped me a lot with git commands. :-)

[Peter230655]

Adjusted the wording of the ValueError, just in case you want to merge this.

[moorepants]

Adjusted the wording of the ValueError, just in case you want to merge this.

It would still need a unit test.

[Peter230655]

Adjusted the wording of the ValueError, just in case you want to merge this.

It would still need a unit test.

Would this be a program without time derivaties so it gives the error? (of course I did test it)
If YES, would I push it with this PR?
Any special 'form' required?
Sorry about all these questions!

[moorepants]

Yes to all three questions.

[Peter230655]

Yes to all three questions.

I wrote a function, which when run will cause the ValueError to be raised - and of course will stop running. (copied it from an existing opty test function and modified according to my needs)

• Would I add it to opty/opty/tests?
• Does this function need to import all the libraries it needs, or just a function which will cause the ValueError to be raised?
• When this function runs, the ValueError is raised and it stops. This is maybe not want you want? But what to do about it? Should it not continue to run if and only if a ValueError is raised?

I found a way (with chatGPT!) so my function stops with an exception if no ValueError is found, If a ValueError occurs it keeps on running.
But on first two points I am unclear.
Thanks!

[moorepants]

I would recommend having a look at the existing unit tests and using the patterns there to come up with how your test will look.

[Peter230655]

I would recommend having a look at the existing unit tests and using the patterns there to come up with how your test will look.
This is what I did, and there I found this:

Would I paste my function at the end of test_direct_collocation.py? (There are many functions there)

[moorepants]

Yes.

[moorepants]

You should use the raises() function from pytest. See where it is used elswhere in the file.

[Peter230655]

Is this how you meant it? I simply copied it from the only other place it is used in test_direct_collocation.py
Frankly unsure how it works.

[moorepants]

You'd have to run the line that raises the error in the with statement.

[Peter230655]

You'd have to run the line that raises the error in the with statement.

Sorry, but I do not understand.

[moorepants]

https://docs.pytest.org/en/4.6.x/reference.html#pytest-raises

[moorepants]

These things are explained in documentation: https://docs.pytest.org/en/4.6.x/reference.html#pytest-raises

[Peter230655]

You ARE a teacher! Your method of not telling me exactly what to do helps me to learn!

Just curiosity: Would my original way have worked and just been very ugly compared to this way, or plain wrong?

[moorepants]

Just curiosity: Would my original way have worked and just been very ugly compared to this way, or plain wrong?

Probably would have worked. We use pytest for the tests, so no reason not to use these helper functions that are more robust than things we write.

[Peter230655]

THANKS!
Stupid question - as often:
Did this merge the test only, or also the change in direct_collocation.py?
(You were not so sure you wanted it)

[Peter230655]

Just curiosity: Would my original way have worked and just been very ugly compared to this way, or plain wrong?

Probably would have worked. We use pytest for the tests, so no reason not to use these helper functions that are more robust than things we write.

Makes sense, plus more 'elegant', I think!

[moorepants]

Everything in the PR is merged. You can see all changes in the "files changed" tab.

[Peter230655]

Everything in the PR is merged. You can see all changes in the "files changed" tab.

Thanks! Makes me feel good to contribute - even if not exactly in gigantic ways! :-)
By the time I will be 75 I hope I will know much more, fewer dumbe questions!° :-)