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

Preference adjustment has been updated to include the managers of both #1122

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gidden
Copy link
Member

@gidden gidden commented Apr 9, 2015

requesters and bidders. This will let institution and region
interactions be based either trader entity. Only bidders now have no
influence on the preference adjustment process, which could be updated
in the future if needed.

There is a notable deprecation of the Agent AdjustPrefs API. As was
true previously, only managers of trading agents are called through
this API, i.e., institutions and regions in the RIF model. The new
AdjustPref API includes the request, bid, preference, and whether the
agent is a manager of the requester or bidder (called
TradeSense). Each trade has its preference adjusted
individually. Anything more complex will require much more difficult
logic in the DRE and in the agent API.

related to cyclus/cyclus.github.com#130 for requester/bidder preference adjustment

@gidden gidden added this to the v1.4 milestone Apr 24, 2015
@gidden
Copy link
Member Author

gidden commented Apr 24, 2015

moving this to 1.4 as there will likely need to be more discussion

@gidden
Copy link
Member Author

gidden commented Aug 19, 2015

I'm hoping for this to go into 1.4 if CEP25 is accepted by then, FYI

@mbmcgarry
Copy link

@gidden - looks like this could use a rebase.

requesters and bidders. This will let institution and region
interactions be based either trader entity. Only bidders now have no
influence on the preference adjustment process, which could be updated
in the future if needed.

There is a notable deprecation of the *Agent* AdjustPrefs API. As was
true previously, only *managers* of trading agents are called through
this API, i.e., institutions and regions in the RIF model. The new
AdjustPref API includes the request, bid, preference, and whether the
agent is a manager of the requester or bidder (called
TradeSense). Each trade has its preference adjusted
individually. Anything more complex will require much more difficult
logic in the DRE *and* in the agent API.
@gidden
Copy link
Member Author

gidden commented Sep 8, 2015

done

On Tue, Sep 8, 2015 at 4:49 PM, mbmcgarry notifications@github.com wrote:

@gidden https://github.com/gidden - looks like this could use a rebase.


Reply to this email directly or view it on GitHub
#1122 (comment).

Matthew Gidden, Ph.D.
Postdoctoral Associate, Nuclear Engineering
The University of Wisconsin -- Madison
Ph. 225.892.3192

@rwcarlsen
Copy link
Member

This new API for adjusting preferences prevents traders/agents from being able to see all bids/reqs before setting any preferences. They have to set preferences based on only knowing a single bid at a time - seems like not what we want. This also is a breaking change even if it doesn't break API - any preference adjustment implemented by traders/archetypes will become completely inoperative since we no longer call the Adjust[Matl/Prod]Prefs functions anymore of anything other than the initial trader itself. This break is arguably not too problematic because we don't have any extra-kernel non-self-trader (i.e. insts, regions) that set prefs, but it is something we should consider carefully.

@gidden
Copy link
Member Author

gidden commented Sep 8, 2015

I'm happy to discuss design changes. This was a first-cut and was done
almost half a year ago.

On Tue, Sep 8, 2015 at 9:02 PM, Robert Carlsen notifications@github.com
wrote:

This new API for adjusting preferences prevents traders/agents from being
able to see all bids/reqs before setting any preferences. They have to set
preferences based on only knowing a single bid at a time - seems like not
what we want. This also is a breaking change even if it doesn't break API -
any preference adjustment implemented by traders/archetypes will become
completely inoperative since we no longer call the Adjust[Matl/Prod]Prefs
functions anymore of anything other than the initial trader itself. This
break is arguably not too problematic because we don't have any
extra-kernal non-self-trader (i.e. insts, regions) that set prefs, but it
is something we should consider carefully.


Reply to this email directly or view it on GitHub
#1122 (comment).

Matthew Gidden, Ph.D.
Postdoctoral Associate, Nuclear Engineering
The University of Wisconsin -- Madison
Ph. 225.892.3192

@gidden
Copy link
Member Author

gidden commented Sep 23, 2015

@rwcarlsen, what if the ExchangeContext (i.e., all information about the current exchange) was added to the function signature?

@rwcarlsen
Copy link
Member

Something like that sounds like a fine enough solution.

@rwcarlsen
Copy link
Member

And to the API semantic breakage, do we know of any archetypes, inventory policies, etc. created by anyone that would break due to the removed calls to adjustmatlprefs?

@gidden
Copy link
Member Author

gidden commented Sep 24, 2015

I know of no such archetypes/policies/etc.

@mbmcgarry
Copy link

I use AdjustMatlPrefs in both my Sink and Enrichment Facility in my RandomSink branch. I didn’t realize this was going to be deprecated - perhaps we can discuss how I can modify my archetypes?

-Meghan

On Sep 24, 2015, at 12:02 PM, Robert Carlsen <notifications@github.commailto:notifications@github.com> wrote:

And to the API semantic breakage, do we know of any archetypes, inventory policies, etc. created by anyone that would break due to the removed calls to adjustmatlprefs?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1122#issuecomment-142989478.

@rwcarlsen
Copy link
Member

@mbmcgarry - do those archetypes use inventory policies?

On Thu, Sep 24, 2015 at 2:44 PM, mbmcgarry notifications@github.com wrote:

I use AdjustMatlPrefs in both my Sink and Enrichment Facility in my
RandomSink branch. I didn’t realize this was going to be deprecated -
perhaps we can discuss how I can modify my archetypes?

-Meghan

On Sep 24, 2015, at 12:02 PM, Robert Carlsen <notifications@github.com
mailto:notifications@github.com> wrote:

And to the API semantic breakage, do we know of any archetypes, inventory
policies, etc. created by anyone that would break due to the removed calls
to adjustmatlprefs?


Reply to this email directly or view it on GitHub<
https://github.com/cyclus/cyclus/pull/1122#issuecomment-142989478>.


Reply to this email directly or view it on GitHub
#1122 (comment).

@mbmcgarry
Copy link

Nope, they do not use inventory policies.

On Sep 24, 2015, at 3:03 PM, Robert Carlsen <notifications@github.commailto:notifications@github.com> wrote:

@mbmcgarry - do those archetypes use inventory policies?

On Thu, Sep 24, 2015 at 2:44 PM, mbmcgarry <notifications@github.commailto:notifications@github.com> wrote:

I use AdjustMatlPrefs in both my Sink and Enrichment Facility in my
RandomSink branch. I didn’t realize this was going to be deprecated -
perhaps we can discuss how I can modify my archetypes?

-Meghan

On Sep 24, 2015, at 12:02 PM, Robert Carlsen <notifications@github.commailto:notifications@github.com
mailto:notifications@github.com> wrote:

And to the API semantic breakage, do we know of any archetypes, inventory
policies, etc. created by anyone that would break due to the removed calls
to adjustmatlprefs?


Reply to this email directly or view it on GitHub<
https://github.com/cyclus/cyclus/pull/1122#issuecomment-142989478>.


Reply to this email directly or view it on GitHub
#1122 (comment).


Reply to this email directly or view it on GitHubhttps://github.com//pull/1122#issuecomment-143034413.


Meghan McGarry, PhD ~ http://mbmcgarry.github.io/
Postdoctoral Fellow, Consortium for Verification Technology
Computational Nuclear Engineering ~ http://cnerg.engr.wisc.edu
Engineering Physics ~ UW-Madison ~ 608-263-6974


@gidden
Copy link
Member Author

gidden commented Sep 24, 2015

I looked in your random_sink branch and didn't see a call to
adjustmatlprefs. Can you provide a link?

Separately, if you are simply doing a check on the the request or bid, it
should be very straightforward (you still have immediate [and easier]
access to both of those).

On Thu, Sep 24, 2015 at 2:44 PM, mbmcgarry notifications@github.com wrote:

I use AdjustMatlPrefs in both my Sink and Enrichment Facility in my
RandomSink branch. I didn’t realize this was going to be deprecated -
perhaps we can discuss how I can modify my archetypes?

-Meghan

On Sep 24, 2015, at 12:02 PM, Robert Carlsen <notifications@github.com
mailto:notifications@github.com> wrote:

And to the API semantic breakage, do we know of any archetypes, inventory
policies, etc. created by anyone that would break due to the removed calls
to adjustmatlprefs?


Reply to this email directly or view it on GitHub<
https://github.com/cyclus/cyclus/pull/1122#issuecomment-142989478>.


Reply to this email directly or view it on GitHub
#1122 (comment).

Matthew Gidden, Ph.D.
Postdoctoral Associate, Nuclear Engineering
The University of Wisconsin -- Madison
Ph. 225.892.3192

@mbmcgarry
Copy link

It’s actually used in the branch random_sink1.3. Here are some links to where I call AdjustMatlPrefs:

enrichment.cchttp://enrichment.cc Line 127https://github.com/mbmcgarry/cycamore/blob/random_sink1.3/src/enrichment.cc
sink.cchttp://sink.cc Line 135https://github.com/mbmcgarry/cycamore/blob/random_sink1.3/src/sink.cc

Thanks,
Meghan

On Sep 24, 2015, at 3:51 PM, Matthew Gidden <notifications@github.commailto:notifications@github.com> wrote:

I looked in your random_sink branch and didn't see a call to
adjustmatlprefs. Can you provide a link?

Separately, if you are simply doing a check on the the request or bid, it
should be very straightforward (you still have immediate [and easier]
access to both of those).

On Thu, Sep 24, 2015 at 2:44 PM, mbmcgarry <notifications@github.commailto:notifications@github.com> wrote:

I use AdjustMatlPrefs in both my Sink and Enrichment Facility in my
RandomSink branch. I didn’t realize this was going to be deprecated -
perhaps we can discuss how I can modify my archetypes?

-Meghan

On Sep 24, 2015, at 12:02 PM, Robert Carlsen <notifications@github.commailto:notifications@github.com
mailto:notifications@github.com> wrote:

And to the API semantic breakage, do we know of any archetypes, inventory
policies, etc. created by anyone that would break due to the removed calls
to adjustmatlprefs?


Reply to this email directly or view it on GitHub<
https://github.com/cyclus/cyclus/pull/1122#issuecomment-142989478>.


Reply to this email directly or view it on GitHub
#1122 (comment).

Matthew Gidden, Ph.D.
Postdoctoral Associate, Nuclear Engineering
The University of Wisconsin -- Madison
Ph. 225.892.3192


Reply to this email directly or view it on GitHubhttps://github.com//pull/1122#issuecomment-143047287.


Meghan McGarry, PhD ~ http://mbmcgarry.github.io/
Postdoctoral Fellow, Consortium for Verification Technology
Computational Nuclear Engineering ~ http://cnerg.engr.wisc.edu
Engineering Physics ~ UW-Madison ~ 608-263-6974


@gidden
Copy link
Member Author

gidden commented Sep 24, 2015

Seems pretty straightforward:

  • sink: always return user_pref
  • enrichment: keep internal state of new_pref. if mass of 235 is 0 return
    -1, else increment new_pref and return new_pref.

On Thu, Sep 24, 2015 at 5:06 PM, mbmcgarry notifications@github.com wrote:

It’s actually used in the branch random_sink1.3. Here are some links to
where I call AdjustMatlPrefs:

enrichment.cchttp://enrichment.cc Line 127<
https://github.com/mbmcgarry/cycamore/blob/random_sink1.3/src/enrichment.cc>

sink.cchttp://sink.cc Line 135<
https://github.com/mbmcgarry/cycamore/blob/random_sink1.3/src/sink.cc>

Thanks,
Meghan

On Sep 24, 2015, at 3:51 PM, Matthew Gidden <notifications@github.com
mailto:notifications@github.com> wrote:

I looked in your random_sink branch and didn't see a call to
adjustmatlprefs. Can you provide a link?

Separately, if you are simply doing a check on the the request or bid, it
should be very straightforward (you still have immediate [and easier]
access to both of those).

On Thu, Sep 24, 2015 at 2:44 PM, mbmcgarry <notifications@github.com
mailto:notifications@github.com> wrote:

I use AdjustMatlPrefs in both my Sink and Enrichment Facility in my
RandomSink branch. I didn’t realize this was going to be deprecated -
perhaps we can discuss how I can modify my archetypes?

-Meghan

On Sep 24, 2015, at 12:02 PM, Robert Carlsen <notifications@github.com
mailto:notifications@github.com
mailto:notifications@github.com> wrote:

And to the API semantic breakage, do we know of any archetypes,
inventory
policies, etc. created by anyone that would break due to the removed
calls
to adjustmatlprefs?


Reply to this email directly or view it on GitHub<
https://github.com/cyclus/cyclus/pull/1122#issuecomment-142989478>.


Reply to this email directly or view it on GitHub
#1122 (comment).

Matthew Gidden, Ph.D.
Postdoctoral Associate, Nuclear Engineering
The University of Wisconsin -- Madison
Ph. 225.892.3192


Reply to this email directly or view it on GitHub<
https://github.com/cyclus/cyclus/pull/1122#issuecomment-143047287>.


Meghan McGarry, PhD ~ http://mbmcgarry.github.io/
Postdoctoral Fellow, Consortium for Verification Technology
Computational Nuclear Engineering ~ http://cnerg.engr.wisc.edu
Engineering Physics ~ UW-Madison ~ 608-263-6974



Reply to this email directly or view it on GitHub
#1122 (comment).

Matthew Gidden, Ph.D.
Postdoctoral Associate, Nuclear Engineering
The University of Wisconsin -- Madison
Ph. 225.892.3192

@gidden
Copy link
Member Author

gidden commented Sep 29, 2015

API has been updated

@rwcarlsen rwcarlsen modified the milestones: v1.4, v1.5 Nov 17, 2015
@scopatz scopatz changed the base branch from develop to master September 7, 2017 18:32
@gonuke gonuke removed this from the v1.6 milestone Oct 17, 2023
@gonuke gonuke added the Feature label Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants