Skip to content
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

CLEM validation is not trapped at start of simulation #9644

Open
lie112 opened this issue Feb 4, 2025 · 4 comments · Fixed by #9645
Open

CLEM validation is not trapped at start of simulation #9644

lie112 opened this issue Feb 4, 2025 · 4 comments · Fixed by #9645
Assignees
Labels
Bug CLEM CLEM specific issues

Comments

@lie112
Copy link
Contributor

lie112 commented Feb 4, 2025

What happened?

While ensuring that the Transmutation.PacketSize provided by the use must be greater than 0, it was also noted that no errors were detected in the _Messages table when identifying validation errors and the simulation was not halted to display validation errors.

What Operating System are you on?

Windows

@lie112
Copy link
Contributor Author

lie112 commented Feb 11, 2025

Hi @par456
relates to #8997

This problem keeps happening and I am yet to fully solve it and it seems to go away for a bit and return.
I believe this also relates to holding onto errors from a previous run when they have actually been addressed.

Here's the rundown

  • I validate properties and write error messages to _messages with summary.writeMessage. This is done in the CLEM events in StartOfSimulation. In my working branch these are called in a method subscribed to "StartOfSimulation" with the messages in Summary cleared to null in "Commencing"

  • This is done before I arrive at ReportInvalidParameters in ZoneCLEM

  • But by the time it gets to ReportErrors the errors have not yet been written to _messages and so there are no errors to report and halt execution and the simulation continues to till it crashes due to an invalid parameter issue further on.

  • ? Am I able to access the Summary messages at this point as I don't really need the database _messages?

  • I had added the summary.WriteMessagesToDatastore() method to try and ensure all was written before I need it.

  • I finally found today that if I place a break point on my ReportErrors or in summary.WriteMessagesToDatastore(), then step through it works, without the breakpoint, it does not find any messages and does not stop.

It seems that even though this is the flow, the messages are not written to the table by the time I call GetMessages() without a breakpoint.

How do you find the DataStore? (I am trying to call the Writer.WaitForIdle as previously mentioned, but not sure this will solve the issue.
I was using for FindInScope from my model but it isn't found. It seems the Simulation model does not have a Parent model defined and so obviously can't keep looking up the tree to Simulations which holds the datastore. How can you find a model outside the simulation space, or do I need to add a [Link] DataStore for this?

It seems very odd that a breakpoint solves my problem but letting it run through doesn't get the messages updated in time to check messages and terminate the simulation.

So, the Summary writes all messages to the datastore in the Completed event.
This is too late for me and explains why they appear after the next fatal exception but not when I need to check them
Using the summary.WriteMessagesToDatastore() doesn't seem to be enough even though the messages are present.

Is there an additional threading issue for the datastore.Writer or something going on.

I wrote a little method to grab the private messages DataTable from Summary Same as GetMessages except it uses the messages DataTable rather than reading from the database. This does in fact given me the details I need so I could work with that if I can add this method with a suitable name, but it would seem very clumsy but avoids any use of the database. I'll see if it is accepted on big push. Noone else will really need to use this. I notice that all the errors are displayed and simulation terminated before the Simulation view in UI completes updating.

@hol353
Copy link
Contributor

hol353 commented Feb 11, 2025

You should be able to put in a [Link] to datastore and then call WaitForIdle.

@par456
Copy link
Collaborator

par456 commented Feb 11, 2025

I was using for FindInScope from my model but it isn't found. It seems the Simulation model does not have a Parent model defined and so obviously can't keep looking up the tree to Simulations which holds the datastore. How can you find a model outside the simulation space, or do I need to add a [Link] DataStore for this?

Edit: Apparently when Simulations are running, they detach from their parent so they are isolated. So that is working correctly.

As Dean says, try using a [Link] tag to get a reference to the Datastore, since that'll get your link before the simulation is detached to be run.

It seems very odd that a breakpoint solves my problem but letting it run through doesn't get the messages updated in time to check messages and terminate the simulation.

That makes sense. Writing to the database happens asynchronously, so when you have that breakpoint it now has enough time in the background to write everything before the next lot of code runs. That means a WaitForIdle should work for solving this problem, we just need to get a DataStore link.

@lie112
Copy link
Contributor Author

lie112 commented Feb 12, 2025

Thanks, that's what I had determined. I guess it's best practice to force the write to the database before checking for errors when needed as these will need to be written to db anyway on Completed event and won't happen twice as the DataTable is set to null after writing in WriteMessagestoDataStore. It seemed logical to access the private messages DataTable from summary (akin to GetMessages, but using messages DataTable rather than SQL call) for this information as it saved the SQL overhead (and going to find the simulation id from the simulation name) and you can be certain that the messages provided didn't need to be filtered by Simulation Name as they belong to the current simulation. BUT, it does get a bit messy to be accessing the private DataStore and you need to know that they will not have been written to the database already and lost (which is ok as long as you do it before Completed event). Asking Summary is also a lot easier when operating in Experiments where tracking the simulation name is more difficult and you can be certain that the errors provided are truly from the current simulation. I had come across even bigger issues when trying to trap these errors in experiments, but this may too resolve themor this might be an issue with threaded experiment simulaitons.

Is there any reason why we couldn't reassign indexes to the MessageType enum? It would have been nice to add various levels to Errors such that they are all errors but might be treated differently in filtering etc and setting enum indexes to non concurrent values can allow later insertion. All errors could have been in the 10 or 100 range with warning in 20, or 200 etc. However, this means the major category needs to be placed in display select statements and it isn't a neat flow from error to information (e.g. if (messageType/10)==1 for errors) or an additional enum with the broad categories needs to be included and we don't need additional fields saved in the database. It would have been nice to add a ValidationError to the enum so I don't have to look for the term validation in the error message to determine the type of error in my validation assessment and the template for displaying these could be customised and the error message didn't need to hold all the validation error text as well. This would also have allowed a range of different warning and information messages to be provided giving the user better flexibility in deciding what message reporting they wanted to ignore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLEM CLEM specific issues
Projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

3 participants