Position implementation change

[bam241]

Position change:
The way the Position is working now, each agent have to define their coordinates (all the cycamore have them) and have there own method to Record them (all have the same), what I did, is add the coordinates and the RecordPosition at the Agent level (each cycamore agent still have to get latitude and longitude on their own), each agent calling the RecordPosition at the Cyclus::Agent::EnterNotify() level, avoiding copy/pasting the RecordPosition method over and over...

I also added a RecordPosition(Agent* agent) method in Position, so the Position knows how to register itself in the database. (this add agent and context include in the Position, if we don't like it, it will be as easy to add the RecordPosition in the agent itself...)
I think this last change make sense, as it allows to keep the Agent class fairly simple and will allow each attribute (Position) to know how to Record himself...

This is redondant with #1508 , but @gonuke advised me to do a separate PR for this change.

[gonuke]

A better way to streamline this without forcing every agent to have a Position (since that's not in the spirit of the toolkit):

1. add RecordPosition() to position.h (as already one here)
2. all agents with a position do the following things
   • inherit from Toolkit::Position
   • #include a special snippet that adds variables with cyclus pragmas in header (e.g. position.cycpp)
   • add Position(latitude,longitude) to the initializer list of their constructor
   • call RecordPosition(this) from EnterNotify()

[katyhuff]

Is this file really intended to be part of the PR?

[bam241]

@katyhuff thx for your review !
To reply to your comments, the idea here is to include the position.cycpp in the different archetype we develop (#include toolkit/position.cycpp) to avoid having to copy paste over and over:

#pragma cyclus var { \
"default": 0.0, \
"uilabel": "Geographical latitude in degrees as a double", \
"doc": "Latitude of the agent's geographical position. The value should " \
       "be expressed in degrees as a double." \
}
double latitude;

#pragma cyclus var { \
"default": 0.0, \
"uilabel": "Geographical longitude in degrees as a double", \
"doc": "Longitude of the agent's geographical position. The value should " \
       "be expressed in degrees as a double." \
}
double longitude;

so adding position on an archetype, one will only have to:

• inherit from Position
• #include position.cycpp whithin the class header
• call the position constructor for the archetype constructor
• call the RecorPosition(this) from the EnterNotify() of the archetype

[bam241]

you can see the related PR draft on cyclus/cycamore#502

[bam241]

@gonuke @katyhuff I believe this is ready for a second review :)

[FlanFlanagan]

@bam241 Are we concerned about the cymetric failures? It seems like the tests are returning larger tables than expected.

[bam241]

@FlanFlanagan I did not investigate this yet, but this is just showing that Cyclus has evolve to something that cymetric don't understand.
So I am not concern about it, but we need to PR a fix for symmetric.
the cymetric_master and the cycamore_master are just there as highlight that the impact of a cyclus change on cycamore and cymetric

[gonuke]

A few requests

[gonuke]

I thinks we need a longer message here that explains how it all works, since it is a brand new concept,

Also, can we use one of the archetypes that’s in cyclus/cyclus to demonstrate?

[gonuke]

What is this for?

[bam241]

this is because cycpp is not run on the file generate by the cpp

see

cyclus/cmake/UseCyclus.cmake

Line 122 in 279a758

SET(ORIG "--pass3-use-orig")

and

cyclus/cli/cycpp.py

Lines 2328 to 2335 in 279a758

	parser.add_argument('--pass3-use-pp', action="store_true", default=True,
	help=("On pass 3, use the preproccessed version of the "
	"original file. This options is mutually exclusive"
	"with --pass3-use-orig."), dest="pass3_use_pp")
	parser.add_argument('--pass3-use-orig', action="store_false",
	help=("On pass 3, use the preproccessed version of the "
	"original file. This options is mutually exclusive"
	"with --pass3-use-pp."), dest="pass3_use_pp")

so when using this snippet method we miss generated code for the additional included variables...
I try to change the --pass3-use-orig to --pass3-use-pp and cyclus is not compiling :(

I don't know what the best solution is...

[bam241]

cc @gonuke this is a follow-up on the conversation we had this morning...

[gonuke]

This needs a news file

[gonuke]

will merge with news file!

[bam241]

shall we not wait for the CEP27 to be ready ?

[gonuke]

This was paused waiting for CEP27 to be accepted. That did happen in March 2021, so this should be reconsidered.

A similar approach, based on CEP27, would be very valuable for @nuclearkatie's #1634